Add a repository hook to enforce new review requirements for changing .webidl files

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

Details

Attachments

(1 attachment)

The DOM module is considering hardening the review requirements for changing .webidl files under mozilla-central and integration branches.  As part of this rule, any patch which touches a .webidl file should have a superreview from a DOM module owner or peer <https://wiki.mozilla.org/Modules/Core#Document_Object_Model>.

We need a hook which checks the commit message for sr=<peer> for the people on the above list if the changeset touches a .webidl file.  I understand that we have an existing binary compatibility hook for .idl files in mozilla-beta so perhaps we could use something like that.
Based on feedback I received from people, the hook should look for r=<peer1>(,<peer2>)* in the commit message instead of sr=<peer>.  I'd be happy to help codify the check if needed.
Created attachment 8412616 [details] [diff] [review]
Patch (v1)

Please note that I basically copied the logic of the hook and the tests from prevent_uuid_changes.py.  The test passes locally.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #8412616 - Flags: review?(ted)
Comment on attachment 8412616 [details] [diff] [review]
Patch (v1)

Review of attachment 8412616 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozhghooks/prevent_webidl_changes.py
@@ +32,5 @@
> +        'khuey',            # Kyle Huey
> +        'jlebar',           # Justin Lebar
> +        'hsivonen',         # Henri Sivonen
> +        'mrbkap',           # Blake Kaplan
> +        'bholley',          # Bobby Holley

Having a hardcoded list here kind of sucks, but whatever floats your boat.

@@ +42,5 @@
> +        # Loop through each file for the current changeset
> +        for file in repo[change_id].files():
> +            # Only Check WebIDL Files
> +            if file.endswith('.webidl'):
> +                webidlReviewed = True

Sort of a weird variable name for this.

::: runtests.py
@@ +881,5 @@
> +    appendFile(join(self.clonedir, "original.webidl"), "interface Foo{};")
> +    add(self.ui, self.clonerepo, join(self.clonedir, "original.webidl"))
> +    commit(self.ui, self.clonerepo, message="original repo commit r=jst")
> +    push(self.ui, self.clonerepo, dest=self.repodir)
> +    print "===== In method", self._testMethodName, " ======="

Can you remove these debug prints?
Attachment #8412616 - Flags: review?(ted) → review+
https://hg.mozilla.org/hgcustom/hghooks/rev/cacacc4b45e0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Blocks: 1002652
Depends on: 1004557
Depends on: 1004558

Updated

3 years ago
No longer depends on: 1004557
Depends on: 1007235

Updated

3 years ago
Depends on: 1005920, 1013925, 1014397

Updated

3 years ago
Depends on: 1032531
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.