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

Adopt better SemVer practices #7563

Closed
ddbeck opened this issue Dec 3, 2020 · 10 comments · Fixed by #7615
Closed

Adopt better SemVer practices #7563

ddbeck opened this issue Dec 3, 2020 · 10 comments · Fixed by #7615
Labels
enhancement 🥇 Nice to have features. question ❔ Issues where a question or problem is stated and a discussion is held to gather opinions.

Comments

@ddbeck
Copy link
Collaborator

ddbeck commented Dec 3, 2020

For "routine" data updates, we usually release as patch versions (i.e., 2.0.z), even if those versions add or remove features. We should probably quit doing this, based on consumer feedback in #7535. But there's more to Semantic Versioning than breaking changes.

I propose we adopt a closer adherence to SemVer practices at the level of features (and above) or browsers (and above). In other words, changes to __compat objects and the immediate children of browsers should be reflected in our release versions. In other words, we’d adopt SemVer for the parts of BCD which aren’t directly described by the schema specifications, plus changes to the schemata themselves.

What does this actually mean?

Here’s some example scenarios:

Things that would trigger a SemVer MAJOR release (e.g., 3.0.0)

  • Removing an existing feature
  • Renaming an existing feature
  • Removing a browser
  • Backwards incompatible changes to the schema (e.g., removing an existing property of a __compat object)

Things that would trigger a SemVer MINOR release (e.g., 2.1.0)

  • Adding a new feature
  • Adding a new browser
  • Backwards compatible changes to the schema. (e.g., adding a new property to a __compat object)
  • Backwards compatible changes that we want to draw attention to (e.g., something that goes through the migrations process)

Things that would trigger a SemVer PATCH release (e.g., 2.0.8)

  • Adding a support statement
  • Removing a support statement
  • Changing a feature’s description
  • Changing the value of a feature’s status

How would our process change?

All this means we’d probably issue more "breaking" releases than we have in the past (though I expect few changes will represent hardships for BCD consumers).

It also means we’d have to do a bit more bookkeeping around releases. More PRs would need to be flagged for release notes and more release notes text would need to be written. This would probably give us the impetus to adopt better practices around release notes. For example keeping BCD’s release notes in the repository (rather than GitHub’s proprietary releases tool) or incrementing BCD’s version number via pull requests rather than commits pushed straight to the default branch.

We would also need to document this for consumers. We should indicate in consumer documentation what’s protected by backwards compatibility and what’s not.

Feedback wanted

What am I missing here? I feel like there may be some other issues presented by this which I haven’t considered yet. I was preparing the release today and kinda got a bit paralyzed by the prospect of changing our release practices unilaterally. I'd love to have your help to make sure we're doing this intentionally.

@ddbeck ddbeck added enhancement 🥇 Nice to have features. question ❔ Issues where a question or problem is stated and a discussion is held to gather opinions. labels Dec 3, 2020
@ddbeck
Copy link
Collaborator Author

ddbeck commented Dec 3, 2020

@chrisdavidmills @Elchi3 @vinyldarkscratch this issue might be of interest. Thanks for your help!

@Elchi3
Copy link
Member

Elchi3 commented Dec 4, 2020

First let me say that the fear of increasing the major version is something I'm very much guilty of bringing into this project. 😅

I also think SemVer should be applied correctly and we should agree on and publish rules for this process, so that it is clear to everyone when we release which update. The rules you outline make sense to me in principal. I think I have one open question:

I think "Removing an existing feature" happens quite a lot given we are identifying a few ghost features and irrelevant features regularly. I don't have a lot of experience with releasing software, so I think my worry was and is that people would be a bit tone deaf if we make new major version increases all the time because some obscure BCD feature entry was removed.
So my question is: If we were to remove or rename very popular feature(s), then that is breaking, but is it also when we (regularly) remove irrelevant features?

Editorial: I was a bit confused by "Adding a support statement" and "Removing a support statement", I think "Updating support statements" summarizes it better?

I feel like there may be some other issues presented by this which I haven’t considered yet.

Probably, but this is a great start and I think if we make this a "release guidelines" doc then we can expand it as time goes by. Much like we do with the data guidelines doc.

@ddbeck
Copy link
Collaborator Author

ddbeck commented Dec 4, 2020

Thanks, @Elchi3. You've given me some helpful things to think on.

Removing an existing feature

I think "Removing an existing feature" happens quite a lot given we are identifying a few ghost features and irrelevant features regularly. I don't have a lot of experience with releasing software, so I think my worry was and is that people would be a bit tone deaf if we make new major version increases all the time because some obscure BCD feature entry was removed.

I was thinking about this too. I've got a lot of thoughts on it, apparently:

The better acquainted I've become with npm packages, the more I think developers must be accustomed to packages shipping "breaking" changes which don't represent particularly dramatic departures from previous releases (e.g., working on stumptown, I saw a number of breaking releases from our dependencies where the "breaking" change was a Typescript definition, only impacting TS users). I suspect shipping these nuisance releases is preferable to something like the surprise of #7535, at very least.

In some ways though, we've created this routine breakage situation for ourselves. We've designed our API (minimal as it is) this way. If we say, if you access api.SomeFeature.someVeryObscureFeature, then you'll get some meaningful data and subsequently take that away, that's a breaking change, obscurity notwithstanding. It's just a fact that our API has lots and lots of names that could break.

We could probably get clever about this to reduce the likelihood of breaking changes. For instance, we could provide something like getDataFor("api.SomeFeature.someVeryObscureFeature") which can handle renames, return useful errors, and or let us emit deprecation warnings for features we've marked for removal. Or do something clever with our schema and proxies (e.g., add a flag for pending removals, emit deprecation warnings on access for a bit, then group the removals themselves into once-monthly or -quarterly releases). But that would require some design and implementation work.

Updating support statements objects

Editorial: I was a bit confused by "Adding a support statement" and "Removing a support statement", I think "Updating support statements" summarizes it better?

Yeah, I was mostly filling out some illustrations to show what would happen when various changes happen, not writing final guidelines, but you caught an important point. What I actually meant to illustrate was that changing a support object (not a support statement) would not be considered breaking. For instance, if you had something like this:

"chrome": {
  "version_added": "23",
  "flags": […]
}

then changing it to something like this:

"chrome": [
  {
    "version_added": "99"
  },
  {
    "version_added": "91",
    "version_removed": "99",
    "flags": […]
  }
]

would be considered a patch change, since any support object is specified to return either an array or an object and can do so at any time, reflecting the facts of the matter.

Interim policy?

Anyway, my failed attempt yesterday to put together a release got me thinking: what should I do right now? Should I continue as if we are not observing SemVer (e.g., breaking things in patch releases), until we've come up with a documented policy? Or should I start observing SemVer while also working on the documentation for it? Or should I go for some hybrid, where we're pushing SemVer minor releases even though they contain SemVer major changes?

@Elchi3
Copy link
Member

Elchi3 commented Dec 7, 2020

In some ways though, we've created this routine breakage situation for ourselves. We've designed our API (minimal as it is) this way.

This is on point. I think the question now is whether we want to enhance our API design to account for this or if we want to communicate better when we break endpoints in the current API. When we break things, there are definitely a few different levels of breakage.

  • Renaming css.* to stylesheets.* probably breaks everything and everyone.
  • Renaming a major subtree with like 30 features (javascript.operators.plus_operator to javascript.operators.plus) breaks a few people's tools.
  • Updating a support object from simple form to array form really shouldn't break anyone because we expect consumers to read BCD's schema and to implement the consuming tool in a way that it can handle both.

So, I don't know, but do the 3 scenarios above map to major, minor and patch in semver? I guess the case that is on the fence is the second, where we could be lax and say it is minor or we are more conservative and announce it as a major change because it can effectively lead to breakage for some people that happen to consume the 30 features out of the 10k or so that we have.

what should I do right now?

I think we should have a look at what is in queue. Is it the nodejs initial version stuff that is hard to categorize with no semver policies right now?

Maybe we merge the UC and QQ browser removal (seems ready to me) and then we have a breaking release anyway?

@ddbeck
Copy link
Collaborator Author

ddbeck commented Dec 7, 2020

it can effectively lead to breakage for some people that happen to consume the 30 features

I think this example gave me some clarity about this. The whole idea of SemVer to is communicate expectations. If a consumers sees a major version change, they should probably read the release notes to find out if they need to make any changes. Most of our users won't be affected by any given change, but it does seem kind to warn about that sort of thing.

Maybe we merge the UC and QQ browser removal (seems ready to me) and then we have a breaking release anyway?

Yeah, this is a good point. It at least buys time to write something formal. 👍

@ddbeck
Copy link
Collaborator Author

ddbeck commented Dec 7, 2020

@Elchi3 and I talked more about this today. We brainstormed on this problem a bit and here's what we came up with:

We should document SemVer-protected names, rather than guaranteeing backwards compatibility for all feature access. Some examples of namespaces we had in mind:

  • api
  • css.at-rules
  • css.properties
  • html.elements
  • javascript.operators

The idea here is to avoid triggering a major release on routine data maintenance (e.g., dropping an irrelevant feature, correcting incorrect capitalization, etc.) while still alerting consumers to large-scale changes (i.e., the problem of #7535, where many features were relocated in a patch release).

I'm going to open in a PR soon (ideally in the next 24 hours), which will document this approach, so we can include it in this week's release.

@robatwilliams
Copy link

As the author of 7535, I think this is on the right track.

The protected names idea seems a reasonable compromise. Too few major releases would break my es-compat tool/linter (or any consumer), while too many would mean its users wouldn't benefit from data completeness/quality improvements until I update the dependency version of the tool and release it.

I did look at the schema documentation (as I expect most serious consumers would need to) when building the tool, so if this list of protected names was unmissable while doing that I would have found it.

@robatwilliams
Copy link

There were three recent patch releases containing breaking changes, as accommodated by the fixes I had to make here: robatwilliams/es-compat#60

This is fine by the stated semantic versioning policy:

You should expect lower-level namespaces, feature data, and browser data to be added, removed, or modified at any time.

This is quite inconvenient, and unless I lock down a specific version (thus opting out of data updates), breaks es-compat for users.

@queengooborg
Copy link
Collaborator

Apologies that an update has broken your package, @robatwilliams, it's never our intention to break a downstream package.

Unfortunately, however, we add and remove features from BCD all the time, to the point where literally every release has at least one feature added or removed. If we were to consider individual features documented in BCD as covered by SemVer's feature addition/removal policies, then almost every new release of BCD would have a major version bump. As we work to refine the data, we've found that certain features were never supported (or haven't been supported in two years and can be considered irrelevant), and other times we've found that the organization of features is not great so we have moved them around to better organize them.

The best recommendation I can make is, in fact, to use a specific patch version, and then use something like Dependabot to automatically update BCD if the unittests pass.

@robatwilliams
Copy link

No worries, thanks for the insight.

I'll consider dependabot and other approaches on the es-compat side to deal with this - robatwilliams/es-compat#61

I wonder what it would look like for the BCD SemVer scope to include all the features, thus eliminating the problem that consumers currently have to deal with. This would also align the "public API" part of the policy with what I expect is the reality of what consumers are using. I see there is now the build-time mirroring feature for related browser families. Perhaps something like that could be used to mirror moved features back to their previous path, and then remove these mirror links in the next major version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🥇 Nice to have features. question ❔ Issues where a question or problem is stated and a discussion is held to gather opinions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants