Reject commit messages that aren't in the right format

[fhd]

Background

In general, all Adblock Plus commit messages should either reference an existing issue by number, or start with Noissue. So far we just hope everyone sticks to that, but it would be better to ensure that in a hook.

What to change

#3674 added a hook that parses the commit message, and since #3676 sets it up as a pretxnchangegroup hook, we can simply change that hook to reject commits with an invalid commit message.

[fhd]

I think this is more complicated than it sounds.

The only hook we seem to be able to use for rejecting changesets is pretxnchangegroup. It works just like changegroup but allows us to reject all changesets. I haven't seen any hook that we can use to reject individual changesets, and I can imagine that that is simply not possible because of the way push works.

So we need to deal with the potential situation where just one changeset in the group was problematic. I suppose the best way of doing that is to check all the issues before attempting to post any comments, but that leaves us with a potential race condition if an issue is being deleted while the hook runs. In that case we would post comments to some of the issues, referencing changesets that don't exist.

I couldn't come up with a good solution for this yet. We may have to finish #3678 first, which should give us more flexibility.

[greiner]

It'd be great if we could also check whether the commit message of Noissue-commits contains a reference (i.e. a URL) to the code review.

[fhd]

Removing the ready flag, I just realised a problem we have to deal with: hg's automatic tag commits. hg tags are actually tracked in a file under version control, so adding or changing a tag needs a commit. That happens automatically, the format is: Added tag $TAG_NAME for changeset $CHANGESET_HASH. I guess the easiest way is to accept that format as valid as well. Thoughts?

Replying to greiner:

It'd be great if we could also check whether the commit message of Noissue-commits contains a reference (i.e. a URL) to the code review.

Yeah, agreed.

[kvas]

Replying to fhd:

Removing the ready flag, I just realised a problem we have to deal with: hg's automatic tag commits. hg tags are actually tracked in a file under version control, so adding or changing a tag needs a commit. That happens automatically, the format is: Added tag $TAG_NAME for changeset $CHANGESET_HASH. I guess the easiest way is to accept that format as valid as well. Thoughts?

Sounds good to me.

[kvas]

I think at this point with most development having moved to GitLab and the remaining few repos to follow soon this ticket can be declared irrelevant. I'm closing it.