Skip to content
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

Update help and remove unused config options from the configuration file #6889

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

jsternberg
Copy link
Contributor

Normalize the output for the various help options so they all follow the
same format and display all relevant options.

Removing some of the unused config options from the configuration file
and updating the help documentation. Removing some remaining references
to clustering within the open source version.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @joelegasse, @gunnaraasen and @e-dard to be potential reviewers

Normalize the output for the various help options so they all follow the
same format and display all relevant options.

Removing some of the unused config options from the configuration file
and updating the help documentation. Removing some remaining references
to clustering within the open source version.
@jsternberg jsternberg force-pushed the js-update-config-options branch from 2cfb3c1 to 22173ac Compare June 21, 2016 19:06
@gunnaraasen
Copy link
Member

LGTM

@e-dard
Copy link
Contributor

e-dard commented Jun 23, 2016

Looks fine to me, but I'm wondering what peoples' thoughts are on deprecating (warning) options first, rather than taking them away, since this change will break anything that are using these options.

@jsternberg
Copy link
Contributor Author

The options removed from the configuration shouldn't be in use anymore. TOML should just ignore any entries that it doesn't know so there shouldn't be any problem with taking away options.

@e-dard
Copy link
Contributor

e-dard commented Jun 23, 2016

@jsternberg sorry I was talking about flags not options. Though looks like you only altered the usage message anyway?

@jsternberg
Copy link
Contributor Author

Ah, that makes a lot more sense. I only removed the help message, but I still define the flag so that existing servers will still work if they use that option. It's just that the option doesn't do anything. Maybe we should add a deprecation message while that value is unset, but I think we can do that in a later release since I don't want to introduce a deprecation message before 1.0. It's not a big deal to just define the flag and ignore the result as long as we don't advertise that it does something (when it doesn't).

Thoughts? Should I add a deprecation message and plan to delete it in a later release?

@jsternberg
Copy link
Contributor Author

I'll merge this as-is and add the deprecation message later if we think it's a good idea.

@jsternberg jsternberg merged commit b8e52ce into master Jun 23, 2016
@jsternberg jsternberg deleted the js-update-config-options branch June 23, 2016 16:49
@timhallinflux timhallinflux added this to the 1.0.0 milestone Dec 19, 2016
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.

5 participants