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

Docs: clarify minor releases and suggest using `~ to version #6804

Merged
merged 2 commits into from Aug 1, 2016

Conversation

hzoo
Copy link
Member

@hzoo hzoo commented Jul 30, 2016

What issue does this pull request address?

#6795 (comment)

I don't think anyone is suggesting the default be 0 or 1 just that any default was set. But according to the policy that will happen. I think we just need to signal that if you don't want a "breaking change / breaking lint build" (in terms of other projects) you should be using ~ in package.json for ESLint since it can be surprising.

@hzoo that's a good point. Want to put together a PR for the readme with that suggestion?

What changes did you make? (Give an overview)

A docs change to the readme to suggest using ~ so users don't get unexpected changes.

Wording can be changed, maybe mention what I said about it being surprising otherwise.

aside: another thing I thought of was when doing minor updates that cause more fixes that we point that out either in the changelog, the summary, or commit message. Then users have more of an idea of what will be breaking (and in those cases we can also explain if theres an easy migration to fix (an autofix for it)

@mention-bot
Copy link

@hzoo, thanks for your PR! By analyzing the annotation information on this pull request, we identified @nzakas, @platinumazure and @alberto to be potential reviewers

@eslintbot
Copy link

LGTM

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.


If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

@eslintbot
Copy link

Thanks for the pull request, @hzoo! I took a look to make sure it's ready for merging and found some changes are needed:

  • The commit summary must be 72 characters or shorter. Please check out our guide for how to properly format your commit summary and update it on this pull request.

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@eslintbot
Copy link

LGTM

@hzoo
Copy link
Member Author

hzoo commented Jul 30, 2016

Weird I guess it was since I used the website to edit and I turned on that private email option on github so the CLA failed, not sure.

@alberto
Copy link
Member

alberto commented Jul 30, 2016

I would clarify that a minor will only report more errors if the case of bugs, when the error should have been reported in the first place.

@@ -166,6 +166,8 @@ ESLint follows [semantic versioning](http://semver.org). However, due to the nat
* An existing formatter is removed.
* Part of the public API is removed or changed in an incompatible way.

Because our policy allows a minor release to result in reporting more errors, it can be seen as a "breaking change". Thus if you don't want an update to ESLint to break your lint build, we recommend you you `~` in `package.json` e.g. `"eslint": "~3.1.0"` in order to only get patch changes.
Copy link
Member

Choose a reason for hiding this comment

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

Let's not use the term "breaking change" here, as I think it will confuse people. It's better to say that any minor update may include more warnings than the previous release. As such, we recommend using the tilde if you want to guarantee the results of your builds.

@nzakas
Copy link
Member

nzakas commented Jul 30, 2016

@alberto that's not strictly true. We may add a new option that we feel is important to be on by default and let people opt-out rather than opting-in.

@eslintbot
Copy link

LGTM

@alberto
Copy link
Member

alberto commented Jul 30, 2016

@nzakas That's news to me. I don't see that discussed in #6244 either

@alberto
Copy link
Member

alberto commented Jul 30, 2016

FWIW, the only conversation I recall us having about this is in #5845 and you seemed to have the opposite opinion.

I'm sorry for being insistent on this, I'm not against it, I'm just trying to understand it, because I am a bit confused now regarding breaking changes in rule options. And I would like to make it very clear in the docs what could make your build fail, so people can choose appropriately.

@hzoo hzoo mentioned this pull request Aug 1, 2016
@nzakas
Copy link
Member

nzakas commented Aug 1, 2016

@alberto the policy says "a new rule option" for minor version. It doesn't specify whether the option is enabled by default or not. And sorry, I don't see where in #5845 I specified a different preference. Can you point it out?

@nzakas
Copy link
Member

nzakas commented Aug 1, 2016

This pull request LGTM. @alberto let's continue the discussion at the TSC meeting.

@nzakas nzakas merged commit 38d0d23 into master Aug 1, 2016
@hzoo hzoo deleted the docs-semver-minor branch August 1, 2016 17:19
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants