938416 - Warn if backout commit messages don't reference changesets

Status: NEW

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Gregory Szorc [:gps]]

We're currently discussing annotating backouts and uplifts on dev.planning. On that thread, Ed Morley said he would like to see commit messages for backouts be of the qbackout form. e.g. "Backed out changeset XYZ" where XYZ is a Mercurial changeset. I don't believe having the Mercurial server issue a "warning" for "suboptimal" commit messages is a contentious change, so I'm filing this bug.

Clients pushing changesets with commit messages not living up to our quality standards should see something like the following:

remote: The following changesets are backouts but have suboptimal commit messages:
remote:
remote:    XYZ Backout bug 123456
remote:
remote: The changesets have been accepted. However, we may eventually reject
remote: changesets with this suboptimal commit message.
remote:
remote: A properly formatted backout commit message references the changeset(s)
remote: that were backed out, a bug number, and the reason for the backout e.g.
remote:
remote:   Backed out changeset ABCDEFGH (bug 123456) for breaking win32 build
remote:
remote: The easiest way to produce valid backouts is to use the qbackout
remote: extension - available from <URL>.

Ehsan was pretty vocal on dev.planning. I'd like his buy-in before we proceed.

Comment 1

[(no longer active)]

How are you suggesting that we should determine whether a changeset is a backout or not?  Doing some kind of a regex on the commit message?

Comment 2

[Gregory Szorc [:gps]]

I think a simple regex test is sufficient.

^(backout|backedout|backed out)

It should be easy to audit commit messages from the wild to see what other patterns we need. A casual inspection says it isn't many.

Comment 3

[(no longer active)]

OK, that seems fine to me.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

The stuff from bug 1366667 fixed this, right?

Comment 5

[Gregory Szorc [:gps]]

Bug 1366667 established strict rules if the changeset touched servo/. I forgot about the existence of this bug when reviewing bug 1366667. It is probably appropriate to apply the strictness rules established in bug 1366667 for the entire repo. We can do that here.

As a stop-gap, we'll want to print a warning message on malformed commit messages before we actually reject them. Let's run that for a few weeks to flush out tools and processes not producing proper commit messages. Then once we're reasonably confident the switch won't be too disruptive, we can flip the switch on rejecting.