Don't require review for pure version bumps

[tromey]

Often I've gotten review for a patch, then had to get separate review to bump the package version and do a release.

However, it seems to me that there is not much benefit to reviewing these version bumps. It adds a bit of friction without any corresponding benefit.

So, I'd like to propose that pure version bumps -- ones only touching the package version -- can be merged without requiring review; provided of course that the normal release procedures are also followed (tagging, publishing to npm).

[nchevobbe]

I agree

[juliandescottes]

I am concerned about "provided of course that the normal release procedures are also followed (tagging, publishing to npm)". The situation is different if we are talking about packages that are solely published on npm and packages that target npm+mozilla-central (reps, source-maps). Not talking about the debugger here, because it is yet another story.

The bump PR that prompted this discussion skipped the release process for source-maps. I commented in the PR to explain why I think we should not do that.

Because I think a consistent releasing process has value, I think reviews have value to encourage following the release process. Therefore, I would prefer to keep reviews required for Reps and source-maps.

For all the other packages, I agree, the review doesn't bring any value :)

[nchevobbe]

I was also thinking about non-mozilla-central-synced packages. For Reps, we follow https://github.com/devtools-html/devtools-core/blob/master/packages/devtools-reps/RELEASE.md which proved to be valuable other time.

[sole]

I agree, in some cases a review to bump a number doesn't add value.

I have two questions:

a) Is this for packages in GitHub or in M-C? I understand you cannot check stuff in without reviews in m-c. b) What are those packages?

Perhaps we can have a 'white list' of packages where we trust people can do a bump+publish without requesting a review. How does it sound?

Also, although it can be a bit far fetched, and we might not need it yet, but we could automate the bump+publish thing in the future. Something like, if you get an r+, send a message to the "DevToolsBot" and it does the thing for you. But maybe we don't need that now.

[juliandescottes]

a) Is this for packages in GitHub or in M-C? I understand you cannot check stuff in without reviews in m-c. b) What are those packages?

The packages we are talking about are the ones in https://github.com/devtools-html/devtools-core/tree/master/packages

They are all maintained on Github. Most of them are published on npm. Most of them are only necessary for tools running in Launchpad (launchpad itself, connectors to interface with Firefox, shims for UI elements normally provided by Firefox etc...).

Two of them (reps and source-map) are also "published" in mozilla-central.

Perhaps we can have a 'white list' of packages where we trust people can do a bump+publish without requesting a review. How does it sound?

Why not! We could also rely on the RELEASE.md present in the package folder. You can find one in both the source-map and reps folders for instance. If a package has no RELEASE.md file in its folder, then you are free to bump without review?

[tromey]

Because I think a consistent releasing process has value, I think reviews have value to encourage following the release process. Therefore, I would prefer to keep reviews required for Reps and source-maps.

I agree that a consistent release process has value. In fact I would like us to be more consistent here, by agreeing to tagging (I think some releases have been done without tags, not sure if that still happens), and by automating the process. My ideal, actually, would be that merging a version-bump PR would automatically tag and npm publish; but I don't know whether that's practical or safe.

However, I don't think the review process actually facilitates this release process. In the past when I've done a bump, the review has just been a delay. I think the situation is usually that one does a version bump in order to do a release -- you're already planning to do that, getting the rubber stamp doesn't add anything.

[juliandescottes]

To be clear, I don't think having reviews facilitate releases. In my mind they help us follow the release process more carefully, simply by knowing that someone will check it.

I haven't done a release in months, so I'm probably disconnected with the reality. If in practice the reviewer simply checks that {new version} is equal to {current version + 1}, then I agree it's useless.

[sole]

An automated script or similar that bumps the version, publishes, and tags would be helpful. There are packages that do that. The writer of the patch could use it right after getting an r+ on the patch, to

1. bump the package version
2. commit "bump to x.y.z"
3. tag it "x.y.z"
4. publish it

Would there be value in looking into that?

[Garbee]

Automatic updating depends on the repository structure. If there are distinct repositories for each component, then something like semantic-release can be used to fully automate release. Developers don't even need to commit/tag the release since it analyzes changes and auto-tags and puts the version into the package on-the-fly. Super nifty to take a load off.

But, if there is a mono-repo with a Lerna JS setup, this fails since it doesn't handle all the individual components. You really need to do manual steps to deploy. However, you can still automate most of this through some bash scripts to happen in CI after a tag is pushed. So, you leave 0.0.0 in package.json like semantic-release, and modify that in CI after tests pass and you go to deploy. That way you aren't messing up the commit history with pure administration tasks.

[juliandescottes]

It looks like the discussion forked a bit here. Automating the release with scripts would definitely be an improvement, maybe it deserves its own RFC (@sole ?). And we can keep this one to be just about reviews for version bumps.

Note that issue #16 also mentions the topic of release automation and tagging.

[bgrins]

From the original proposal:

So, I'd like to propose that pure version bumps -- ones only touching the package version -- can be merged without requiring review; provided of course that the normal release procedures are also followed (tagging, publishing to npm).

I agree with this. If the commit is just bumping a version # in package.json, then a review is more or less a rubber stamp and the normal release procedures (tagging, publishing to npm) are presumably going to happen after that commit.

For m-c commits we have the concept of r=me which is what is being proposed here. We also have a concept of 'rubber stamp' where you can get consent over irc or elsewhere instead of requesting review and put rs=name instead of r=name. IMO either would be acceptable here and preferable over requesting and waiting for a review for a number change in package.json.

The benefit that a reviewer could have is an opinion on what version should be published (i.e. we are attempting a minor version bump but due to some breaking change we should really be doing a major bump, or vice versa). But I'm inclined to trust the judgement of the person making the change and limit the amount of process - there's already a lot of process currently to get our code back and forth between m-c and github.

[juliandescottes]

This RFC was accepted: if a repository owner wants to update the package version without requesting review, they can do it.

As mentioned in the comments, the reviewer often lacks context to check anything beyond the simple version number change, making the review useless. Plus, the process is already complicated enough for a very small number of consumers.

Many comments were about the release process and how to improve/automate it. This is a more complex discussion which should live in its own RFC. Does anyone here feel like creating a RFC dedicated to this? Maybe @sole @tromey @nchevobbe ?

[juliandescottes]

I am closing this as no further action is required. If anyone feels like working on package bump automation, feel free to move forward.