Skip to content

Conversation

@rjernst
Copy link
Member

@rjernst rjernst commented Dec 14, 2016

This change renames and changes the behavior of SettingCommand to have
its primary method take in a fully initialized Environment for
elasticsearch instead of just a map of settings. All of the subclasses
of SettingCommand already did this at some point, so this just removes
duplication.

This change renames and changes the behavior of SettingCommand to have
its primary method take in a fully initialized Environment for
elasticsearch instead of just a map of settings. All of the subclasses
of SettingCommand already did this at some point, so this just removes
duplication.
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@jasontedor
Copy link
Member

The concept is right, there's a failing test that is clearly related to this PR but I trust that you'll address. Fire at will.

@rjernst
Copy link
Member Author

rjernst commented Dec 19, 2016

Thanks @jasontedor. I fixed the failing test, and check passes for me now locally. Jenkins seems to still be having unrelated git issues; I'm going to push.

@rjernst rjernst merged commit 850f51d into elastic:master Dec 19, 2016
@rjernst rjernst deleted the refactor_env_commands branch December 19, 2016 23:23
rjernst added a commit that referenced this pull request Jan 6, 2017
* Internal: Refactor SettingCommand into EnvironmentAwareCommand

This change renames and changes the behavior of SettingCommand to have
its primary method take in a fully initialized Environment for
elasticsearch instead of just a map of settings. All of the subclasses
of SettingCommand already did this at some point, so this just removes
duplication.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants