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

Should .Default(false) be used on all boolean options? #2865

Closed
kevina opened this issue Jun 18, 2016 · 6 comments
Closed

Should .Default(false) be used on all boolean options? #2865

kevina opened this issue Jun 18, 2016 · 6 comments
Labels
need/community-input Needs input from the wider community

Comments

@kevina
Copy link
Contributor

kevina commented Jun 18, 2016

@RichardLitt has been adding .Default(false) to a lot of Boolean options. To me this seams really unnecessary and adds noise to the help text. I can find very little president for this in the help text for most (unix) command line options.

The only discussion I can find on this is in the comments for pull requests #2582 where @whyrusleeping and @jbenet commented on it. However, @jbenet final comment seamed more about if Boolean options should always default to false and less on if "Default: false" should be displayed for each and every Boolean option that defaults to false.

This issue is a request to formally document the policy and the reason behind it.

@kevina
Copy link
Contributor Author

kevina commented Jun 18, 2016

For the record I should add that I think "Default: true" should be displayed for the few Boolean options that default to true, but "Default" false" is unnecessary and redundant.

@whyrusleeping
Copy link
Member

@kevina I agree. I think adding the Default(false) is just noise and have voiced this a few times.

@RichardLitt
Copy link
Member

Yeah, I took @jbenet's comment to mean I should just put it in. I'm happy to remove it if is clearer, it is some noise and we can assume that they will be false. Adding Default: true if valid is something that should be done, for sure.

@kevina kevina added the need/community-input Needs input from the wider community label Aug 24, 2016
@kevina kevina changed the title Document policy on .Default(false) for boolean options. Should .Default(false) be used on all boolean options? Aug 24, 2016
@kevina
Copy link
Contributor Author

kevina commented Aug 24, 2016

@whyrusleeping I don't think this issue was ever resolved or properly discussed. I am reopening. I also changed the title to be more direct.

@kevina kevina reopened this Aug 24, 2016
@RichardLitt
Copy link
Member

AFAIK, this has been discussed a lot off-GitHub. The resolution we came to is that, no, .Default(false) is an understood quality of booleans, and it doesn't mean to be made explicit. If you see this in the code, you can remove it, and there's no expectation going forward to include it.

@whyrusleeping
Copy link
Member

Cool, i think we're in agreement here that Default(false) is not necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/community-input Needs input from the wider community
Projects
None yet
Development

No branches or pull requests

3 participants