Skip to content

Remove unused config - #5564

Merged
gianm merged 2 commits into
apache:masterfrom
niketh:remove-unused-config
Apr 3, 2018
Merged

Remove unused config#5564
gianm merged 2 commits into
apache:masterfrom
niketh:remove-unused-config

Conversation

@niketh

@niketh niketh commented Apr 2, 2018

Copy link
Copy Markdown
Contributor

@gianm I understand that CoordinatorDynamicConfig allows dynamic configurations. But, I think we can combine DruidCoordinatorConfig.java and CoordinatorDynamicConfig.java.

@niketh
niketh force-pushed the remove-unused-config branch from 61e10d3 to 0ad8cfd Compare April 2, 2018 22:40

@gianm gianm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I was surprised to learn that these configs are not being checked here, but are bound some other place.

@jihoonson

Copy link
Copy Markdown
Contributor

Please check the CI failure.

@niketh
niketh force-pushed the remove-unused-config branch from 18a62c7 to ac00f44 Compare April 3, 2018 01:22
@niketh

niketh commented Apr 3, 2018

Copy link
Copy Markdown
Contributor Author

@gianm @jihoonson Also do you like the idea of combining DruidCoordinatorConfig and CoordinatorDynamicConfig ?

@gianm

gianm commented Apr 3, 2018

Copy link
Copy Markdown
Contributor

@niketh How would you suggest combining them? It isn't clear to me how it would be done, since one is updated dynamically and one is based on static configuration.

@gianm
gianm merged commit f0a94f5 into apache:master Apr 3, 2018
@dclim dclim added this to the 0.13.0 milestone Oct 8, 2018
riovic918data pushed a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
* Remove unused config

* Fix failing tests
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.

4 participants