-
Notifications
You must be signed in to change notification settings - Fork 178
feat(config): default configuration with merge support for downstream overrides #396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(config): default configuration with merge support for downstream overrides #396
Conversation
… overrides
Creates the required infrastructure for downstream forks to be able to provide
default config overrides without modifying the original source code.
Downstream forks should be able to create merge/rebase scripts that
automatically accepted downstream merge conflicts in config_default_overrides.go
since this file will never change upstream.
Example usage for downstream forks:
To customize defaults, simply populate fields in the returned StaticConfig:
```go
func defaultOverrides() *StaticConfig {
return &StaticConfig{
ListOutput: "json", // Override default list output format
Toolsets: []string{"core"}, // Override default enabled toolsets
Port: "9000", // Override default port
}
}
```
Any fields specified here will override the base defaults defined in config_default.go.
Fields not specified will preserve their base default values.
Signed-off-by: Marc Nuri <[email protected]>
Skips toolset metadata tests in case there are config overrides. This is useful in downstream forks where the default toolsets might be different to those upstream. Signed-off-by: Marc Nuri <[email protected]>
| overrides := defaultOverrides() | ||
| mergedConfig := mergeConfig(defaultConfig, overrides) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this approach
| } | ||
|
|
||
| func (s *ToolsetsSuite) TestDefaultToolsetsTools() { | ||
| if configuration.HasDefaultOverrides() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is a good hook, for downstream.
Cali0707
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I really like this approach @manusa
| return StaticConfig{ | ||
| // IMPORTANT: this file is used to override default config values in downstream builds. | ||
| // This is intentionally left blank. | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great @manusa !
Creates the required infrastructure for downstream forks to be able to provide default config overrides without modifying the original source code.
Downstream forks should be able to create merge/rebase scripts that automatically accepted downstream merge conflicts in config_default_overrides.go since this file will never change upstream.
Example usage for downstream forks:
To customize defaults, simply populate fields in the returned StaticConfig:
Any fields specified here will override the base defaults defined in config_default.go. Fields not specified will preserve their base default values.
The second commit fixes #391 (AFAIU)
The full toolset metadata tests are skipped in case there are config overrides.
If downstream follows the convention:
The tests should not fail.