1002652 - Please deploy the prevent_webidl_changes hook

Status: RESOLVED FIXED

(Developer Services :: General, task)

Description

[(no longer active)]

This hook was added in bug 1001106.  We need it on mozilla-central and all of its integration branches, mozilla-aurora and mozilla-beta.

Thanks!

Comment 1

[Kendall Libby [:fubar] (he/him)]

Just so I'm clear, is that "and all of its integration branches, which are m-a and m-b" or is "all of its integration branches" another list of repos? 

Also, changeset cacacc4b45e0, correct?

Comment 2

[Hal Wine [:hwine] use NI!]

Ehsan -- it sounds like we would want this on all project branches as well. Can you confirm? Also, can you think of any repos where this hook would cause a problem. Given that this is new policy, I'd rather "over apply" than miss one.

fubar: this will be all of the repos in integration/releases-*, projects/*, and mozilla-central. (neither m-a nor m-b)

Comment 3

[(no longer active)]

(In reply to Kendall Libby [:fubar] from comment #1)
> Just so I'm clear, is that "and all of its integration branches, which are
> m-a and m-b" or is "all of its integration branches" another list of repos? 

I meant mozilla-inbound, b2g-inbound and fx-team.  I think these are the only ones.

> Also, changeset cacacc4b45e0, correct?

Yep.

(In reply to Hal Wine [:hwine] (use needinfo) from comment #2)
> Ehsan -- it sounds like we would want this on all project branches as well.
> Can you confirm? Also, can you think of any repos where this hook would
> cause a problem. Given that this is new policy, I'd rather "over apply" than
> miss one.

Hmm, project branches usually have their own review policies (and sometimes they don't even require review) so I would say we don't want this to be enabled on any project branches.

Comment 4

[(no longer active)]

ping?

Comment 5

[Kendall Libby [:fubar] (he/him)]

I'm actually more uncertain about which repos you want this hook on now, between #c0, #c2, and #c3.  Sorry if this is a lot of back and forth, but I don't know how all repos are used where, and by whom, and for what, so I need a clear, explicit list.

Comment 6

[Hal Wine [:hwine] use NI!]

Fair enough! Let's go with this:
 mozilla-central
 integration/mozilla-inbound
 integration/b2g-inbound
 integration/fx-team

Comment 7

[Kendall Libby [:fubar] (he/him)]

Ok, added:

pretxnchangegroup.d_webidl = python:mozhghooks.prevent_webidl_changes.hook

Shout if something seems amiss or if you want to add it to other repos.

Comment 8

[(no longer active)]

Thanks, Kendall!  Can you please add mozilla-aurora, mozilla-beta and mozilla-release as well?  I'm trying to figure out an answer as to what we should do for project branchers...

Comment 9

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

As far as I can see, nothing in the merge from mozilla-inbound to mozilla-central that I'm attempting to push tonight seems to touch webidl files, but I'm getting the following error when I make that attempt:
WebIDL file dom/webidl/WorkerNavigator.webidl altered in this changeset without DOM peer review

When I run | hg log dom/webidl/WorkerNavigator.webidl | 
The most recent change was in February:
changeset:   181047:c5624c2a2d2f
user:        Gene Lian <[e-mail]>
date:        Mon Feb 24 21:57:15 2014 +0800
summary:     Bug 949325 - C++ wrapper to support DataStore API on the worker (part 2-3, dispatch tasks on the worker to

changeset:   162615:df0f58eb2b6f
user:        Nikhil Marathe <[e-mail]>
date:        Tue Nov 19 15:08:50 2013 -0800
summary:     Bug 925437 - Implement navigator.onLine on Workers. r=khuey sr=sicking

changeset:   156577:bdd0b739b34d
user:        Tareq Khandaker <[e-mail]>
date:        Wed Nov 20 09:29:01 2013 -0500
summary:     Bug 925847 - WorkerNavigator does not implement all of NavigatorID. r=nsm

changeset:   139669:4e044202c1b4
user:        Ms2ger <[e-mail]>
date:        Wed Jul 24 09:38:23 2013 +0200
summary:     Bug 878401 - Move WorkerNavigator to WebIDL; r=khuey






Is this something screwy with Windows file systems or the command prompt maybe? Explorer says that .webidl was modified sometime today, but hg doesn't seem to agree.

Comment 10

[Hal Wine [:hwine] use NI!]

changing ni to the hook developer

Comment 11

[Hal Wine [:hwine] use NI!]

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #8)
> Thanks, Kendall!  Can you please add mozilla-aurora, mozilla-beta and
> mozilla-release as well?  I'm trying to figure out an answer as to what we
> should do for project branchers...

Moved additional branches to bug 1004368 to avoid any potential impact to chem spills until the impact on sheriffs & merging is fully understood.

Comment 12

[Ed Morley [:emorley]]

Sounds like we need the new hook to skip changesets with merge in the name

Comment 13

[Ed Morley [:emorley]]

On another merge:
pushing to ssh://hg.mozilla.org/mozilla-central/
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 113 changesets with 818 changes to 424 files
remote:
remote:
remote: ************************** ERROR ****************************
remote:
*** WebIDL file dom/webidl/WorkerNavigator.webidl altered in this changeset without DOM peer review***
remote:
Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote:
remote: *************************************************************
remote:
remote:
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.d_webidl hook failed

Comment 14

[Ed Morley [:emorley]]

Kendall, please may you disabled the hook on all repositories it is updated, since it's blocking us pushing merges, unless we spam the commit messages.

Comment 15

[Kendall Libby [:fubar] (he/him)]

Done.

Comment 16

[Ed Morley [:emorley]]

Super quick - thank you :-)

Comment 17

[(no longer active)]

The hook just caught bdd0b739b34d, which modified Navigator.webidl without a DOM peer review.  Is there really anything related to merge commits here?

Perhaps we should first enable the hook on mozilla-inbound, b2g-inbound and fx-team, wait a day or so, and then enable it on mozilla-central.

Comment 18

[(no longer active)]

Oh hmm, nevermind, didn't pay attention to the dates :(

Comment 19

[Ed Morley [:emorley]]

Please could we also add an "OVERRIDE WEBIDL HOOK" or similar? For two of the merges today I had to pick an arbitrary reviewer to add to the commit message.

Comment 20

[Hal Wine [:hwine] use NI!]

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #17)
> Perhaps we should first enable the hook on mozilla-inbound, b2g-inbound and
> fx-team, wait a day or so, and then enable it on mozilla-central.

Can we let this hook ride the trains w.r.t. m-a, m-b, & m-r?

Comment 21

[(no longer active)]

(In reply to Ed Morley [:edmorley UTC+0] from comment #19)
> Please could we also add an "OVERRIDE WEBIDL HOOK" or similar? For two of
> the merges today I had to pick an arbitrary reviewer to add to the commit
> message.

No, we should just deploy it on integration branches first.

(In reply to Hal Wine [:hwine] (use needinfo) from comment #20)
> (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> #17)
> > Perhaps we should first enable the hook on mozilla-inbound, b2g-inbound and
> > fx-team, wait a day or so, and then enable it on mozilla-central.
> 
> Can we let this hook ride the trains w.r.t. m-a, m-b, & m-r?

No need to do that, bug 1004558 fixes the issue with merge commits.

Comment 22

[(no longer active)]

Kendall, can you please deploy the hook as of changeset 723018c7baf1 to mozilla-inbound, b2g-inbound and fx-team please?  Thanks!

Comment 23

[Kendall Libby [:fubar] (he/him)]

All set! Shout or ping me on IRC if you see something amiss.

Comment 24

[(no longer active)]

Thanks!

Comment 25

[(no longer active)]

Kendal, do you mind deploying 9ff49284a16d to mozilla-inbound, b2g-inbound and fx-team please, when you get a chance?  I fixed a problem in the error message generated by the hook.  Thanks!

Comment 26

[Kendall Libby [:fubar] (he/him)]

All set!

Comment 27

[(no longer active)]

I'd say we're ready to deploy this to mozilla-central as well.

Comment 28

[Kendall Libby [:fubar] (he/him)]

enabled on m-c.

Comment 29

[(no longer active)]

Thanks, so I think we're done in this bug.  I'll ping on bug 1004368 next week to allow this hook to bake on trunk for a few days.