Formalize semantic versioning policy

[nzakas]

Continuing from #6176 (comment), JSCS had a very clear semantic versioning policy on its README (https://github.com/jscs-dev/node-jscs/blob/master/OVERVIEW.md#user-content-versioning--semver) that made it obvious when something should be a breaking change or not. We should come up with a similar statement.

[alberto]

Related: #6240

[btmills]

Copying over the JSCS policy and adding my own comments in italics. For reference, currently Breaking: is the only major-level tag, New: and Update: are minor-level, and all other tags are patch-level.

• Patch release:
  • A bug fix in a rule that causes JSCS ESLint to report less fewer errors.
    Currently Fix:.
  • Docs, refactoring, and other "invisible" changes for user.
    *Currently Chore:, Docs:, Build:, and Upgrade.
• Minor release:
  • Any preset changes.
    We've treated changes to eslint:recommended (or previously the default ruleset) as Breaking: before.
  • A bug fix in a rule that causes JSCS ESLint to report more errors.
    Currently Fix:. Previously we've allowed these changes in patch releases under the reasoning that the new behavior was the original intention as specified in the rule. I think, given the install base and that these changes might break somebody's CI, that it makes sense to make these minor-level changes.
  • New rules or new options for existing rules that don't change existing behavior.
    Currently New: and Update:.
  • Modifying rules so they report less fewer errors, and don't cause build failures.
    I'm not coming up with examples for this off the top of my head. Rule conflicts maybe? We've treated those as Fix: before.
• Major release:
  • Purposefully modifying existing rules so that they report more errors or change the meaning of a rule.
    Based on Docs: Add rule deprecation section to user guide (fixes #5845) #6201, for the time being, we might add new rules to replace old ones if meaning changes, but the old ones won't go away, so we don't have to worry about this?
  • Any architectural changes that could cause builds to fail.
    Typically we do this in a backwards-compatible way that introduces new behavior as an alternate form. This could cover the case where we remove the old way.

[nzakas]

Trying to kickstart this again. I think we basically want to adopt what JSCS has. Here's my proposal:

• Patch release
  • A bug fix to the CLI or core (including formatters).
  • A bug fix in a rule that results in ESLint reporting fewer errors.
  • Improvements to documentation.
  • Non-user-facing changes such as refactoring code, adding, deleting, or modifying tests, and increasing test coverage.
  • Re-releasing after a failed release (i.e., publishing a nonfunctional release).
• Minor release
  • A bug fix in a rule that results in ESLint reporting more errors.
  • A new rule is created.
  • An existing rule is deprecated.
  • A new option to an existing rule.
  • A new CLI capability is created.
  • New capabilities to the public API are added (new classes, new methods, new arguments to existing methods, etc.).
  • A new formatter is created.
• Major release
  • An existing rule is removed.
  • An existing formatter is removed.
  • eslint:recommended is updated.
  • Part of the public API is removed or changed in an incompatible way.

@eslint/eslint-team thoughts?

[pedrottimark]

Agree with principle adopt JSCS. It looks clear. A few tactical observations or suggestions:

Patch

• move “A bug fix in a rule…” to first item for symmetry with Minor
• reword “Re-releasing after a failed release…” nonfunctional = what, completely hosed?

Minor

• move “A new option…” to second or third item

Major

• maybe move eslint:recommended to first item

[hzoo]

@pedrottimark I see so moving the bullet points based on familiarity/frequency in changes?

Do we want an explicit point for adding/fixing autofix to a rule or that does under "a rule"?

[pedrottimark]

Yes, a guideline is organize information to make it clear and quick to interpret both ways:

• in Patch/Minor/Major release, what kind of change is possible?
• for a kind of change, is it possible in Patch/Minor/Major? Therefore, though obvious to many, maybe some simple wording that Minor includes Patch and Major includes Minor. Rock, Paper, Scissors.

Change from non-fixable to fixable seems important to make explicit.

You who are more experienced than me can say about changes to amount of fixes. Is there any analogy to reporting more or fewer errors?

[mikesherov]

👍 on adopting HSCS rules here.

[ilyavolodin]

What's an HSCS rules? Sorry, not familiar with it.

[nzakas]

I'm pretty sure he meant JSCS and just typed it wrong :)

[nzakas]

@pedrottimark I think making a rule fixable is a minor release. Same with making it non-fixable. Fixes are not guaranteed to be applied, so there's no real harm in adding or removing fixes.

[mikesherov]

Adding a fix should be minor. The principle for major is "will this change make CI tools fail where they were passing before". For adding fixers, the answer is no.

[alberto]

I'm ok with everything except with:

• (minor) A bug fix in a rule that results in ESLint reporting more errors.

Semver states bugfixes not changing the API should be patch releases. If a rule catches an error we were supposed to be catching that's a good thing. That's the difference between a bug and an enhancement.

[mikesherov]

@alberto Semver can't be strictly followed in this case, because we can't universally call "randomly" breaking builds a feature. There are many users of linters who expect ~ to mean that the build will never suddenly break, and ^ to mean the build will break only when previously uncaught errors that should've been caught are now caught.

[alberto]

I wouldn't call it random. If we release a patch release which detects an error that should have been detected before, I think their build will be failing for a good reason.

[nzakas]

@alberto i had the same initial reaction and had to do a lot of soul searching to get comfortable with it. I do think there's something to @mikesherov's point that there is an expectation that patch releases won't cause a passing build to fail. That, to me, is why requiring a minor release when warnings increase makes sense.

[ilyavolodin]

What happens if we can't determine if the number of errors will increase/decrease? For example, rule was flagging something that shouldn't have been flagging, but at the same time not flagging actual errors?

[nzakas]

@ilyavolodin that would be a minor release. It's not really the number of warnings, it's the number of things being flagged. Whenever something wasn't flagged and now will be flagged, that's a minor release.

[markelog]

Semver states bugfixes not changing the API should be patch releases

It doesn't seem to contradict semver fyi -

Patch version Z (x.y.Z | x > 0) MUST be incremented if only backwards compatible bug fixes are introduced. A bug fix is defined as an internal change that fixes incorrect behavior.

So in this context, we can define them as "features" or even backwards incompatible bug fixes which do not change public API, that is debatable of course.

This point is interesting actually, many people in the JSCS team were weird out by this when it was introduced, now it is hard to imagine otherwise, since this tactic worked out pretty well

[ilyavolodin]

Just an FYI, currently our release script determines what type of release this is going to be based on the commit messages. If we split up Fix to be either patch or minor based on the fix itself, we will not be able to rely on the script anymore (not a big deal, and shouldn't be blocking this discussion, but just something to keep in mind).

[nzakas]

@ilyavolodin I think what we're saying here is that "Fix:" is only used when we're causing a rule to not flag something it was previously flagging. For everything else, we'd use "Update:".

[nzakas]

I'm going to throw together a PR. It seems like we're agreeing on most of the details, so I think we can work out any further issues on the PR.

[nzakas]

So this is an interesting test of the rule fix/update argument: #6324

The rule was not flagging something that it clearly should have, so we labeled this as a bug. "Fixing" the bug means it will now work properly, though the result is that it's flagging something it was meant to flag before but wasn't. Is that a patch change or a minor change?

[mikesherov]

Yeah, it seems weird. But it's a minor in that case.

Mike Sherov

On Jun 8, 2016, at 3:38 PM, Nicholas C. Zakas notifications@github.com wrote:

So this is an interesting test of the rule fix/update argument: #6324

The rule was not flagging something that it clearly should have, so we labeled this as a bug. "Fixing" the bug means it will now work properly, though the result is that it's flagging something it was meant to flag before but wasn't. Is that a patch change or a minor change?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[alberto]

I don't think that case is special. If we want to avoid breaking anyones build, we can tell ourselves that is an enhancement and call it a day. If we want people to get this fixed ASAP, we should label it as a bug and let their CI tell them there is a problem with their build. This is not a critical bug, but what if we find something missing from one of our "Possible errors" rules? Wouldn't you want your build to fail if there was something wrong with your code there?

If someone doesn't want to get failures because fixed bugs, the should pin eslint version or make it not fail their CI, IMO.

[mikesherov]

Finding more lint errors would be an enhancement here. Telling them to pin is harder because npms default behavior is ^, which does force their CI to catch new enforcements. ~ allows them to have green build guarantee but still get bug fixes.

[BYK]

I think I'm with @mikesherov and @nzakas with this one. If you broke my already working and passing build with a patch update because it now catches a lint error that should have been caught before I'd curse you for making me drop everything else and try to fix the "stupid" thing. I know it is not stupid but I'm pretty sure that's how I will feel when this happens to me :)

[nzakas]

Okay, it seems like we have a good consensus to move forward with this.

Just to be clear: from now on, any changes to a rule that will result in something being flagged that wasn't being flagged before will have an "Update:" commit message instead of "Fix:". I'd suggest we still flag such instances as "bug" on the issue to indicate there was a logic error rather than saying we're doing a formal enhancement.

Everyone good with that?

[platinumazure]

@nzakas That would require the developer guide to be updated to indicate that "bug" label !== "Fix:" prefix. Could we maybe do "bug"/"enhancement" labels (as a pair) to make those cases clearer?

[nzakas]

Yeah, that sounds like a good compromise.

[nzakas]

I've updated the pull request to include changes to maintainer/developer guides