Skip to content

Conversation

@manusa
Copy link
Member

@manusa manusa commented Nov 18, 2025

Fixes #470

This might imply breaking changes downstream if there are references to the interfaces.

I think the breaking changes tradeoff is OK because there is now a single simple config.Extended interface which avoids code duplication.
This will also allow to better reuse the pattern for other config extensions if needed.

@manusa manusa added this to the 0.1.0 milestone Nov 18, 2025
@manusa manusa requested review from Cali0707 and matzew November 18, 2025 10:30
Copy link
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this cleanup!

@manusa
Copy link
Member Author

manusa commented Nov 19, 2025

Hi @Cali0707

Addressed the comments regarding ExtendedConfigRegistry and making it private.

I didn't change #475 (comment) yet.
I don't mind changing if you still consider it unclear with regards to my comment (config.Extended should be clearer than config.ExtendedConfig IMHO).
Please, let me know about the last point and we'll proceed accordingly.

@manusa manusa requested a review from Cali0707 November 19, 2025 10:09
@manusa manusa force-pushed the feat/extended-config branch from e99ef87 to 5000e9e Compare November 21, 2025 05:29
@manusa manusa force-pushed the feat/extended-config branch from 5000e9e to f47e6ff Compare November 21, 2025 05:51
Copy link
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for this cleanup @manusa

I'll leave it up to you to decide when to merge this, since as you note it may break some things downstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CONFIG] Unify implementations for ProvierConfig and ToolsetConfig

2 participants