Sheriffing comm-central
Hi all,
as you know, some of us have Level 1 access rights, that is, they can push to try repositories, and others have Level 3 access rights, they can push to (mostly) all repositories, including comm-central, comm-beta, comm-esr68, and even mozilla-central (don't! - you get into trouble).
Traditionally people have pushed their own patches to comm-central, but this practise has mostly stopped now for various reasons.
In the olden days, people just pushed stuff and then walked away as the tree was mostly completely orange anyway. Back then, Aleth came around and slapped people on the fingers when they landed bustage ;-) - Landings weren't coordinated with M-C merges, so it was hard to find regressions since between two C-C pushes there could have been any number of M-C merges. The tree was frequently closed for days or even weeks due to bustage.
As there are many people working on C-C code now who are relying on reliable try results, all this has completely changed. These are the new rules:
- There is now a dedicated sheriffing service, that means, patches are landed at defined times.
- Test failures will not be tolerated, anything that breaks a test will be backed out not to affect others.
- Most people, including those with Level 3 rights, set "checkin needed" on the bug and the sheriff will take care of landing the patch. Mostly I land patches after the following M-C merge times (all GMT): 10 AM, 4 PM and 10 PM, Geoff lands after the 4 AM merge.
- Of course developers with Level 3 rights can still land their own patches, especially large patches or multi-part ones, but it's appreciated to supply patches to the "checkin needed" pool for use at the specified times.
Some other measures of interest:
- Failing tests which can't be fixed via a backout will be disabled and marked especially in BMO.
- Any other bustage, like compile or build bustage, will usually be fixed within a few hours after occurring, typically with patches that get the so-called PLR (post landing review).
All this is done to enable others to build and run successful try runs at any time. The new rules have been in place since 2017 and have worked very well.
Jörg.
Dear Jörg,
I would like to thank you and others who have kept C-C tree in good
shape for a long time by now.
I have noticed that there are much less collisions between uncoordinated
M-C and C-C changes these days. : This means I should get BOTH M-C and
C-C updated together to
get the benefit of the recent tree changes. This was not quite the case
a few years ago.
Thank you !
Chiaki
On 2019/10/10 8:09, Jörg Knobloch wrote:
Hi all,
as you know, some of us have Level 1 access rights, that is, they can
push to try repositories, and others have Level 3 access rights, they
can push to (mostly) all repositories, including comm-central,
comm-beta, comm-esr68, and even mozilla-central (don't! - you get into
trouble).
Traditionally people have pushed their own patches to comm-central,
but this practise has mostly stopped now for various reasons.
In the olden days, people just pushed stuff and then walked away as
the tree was mostly completely orange anyway. Back then, Aleth came
around and slapped people on the fingers when they landed bustage ;-)
- Landings weren't coordinated with M-C merges, so it was hard to find
regressions since between two C-C pushes there could have been any
number of M-C merges. The tree was frequently closed for days or even
weeks due to bustage.
As there are many people working on C-C code now who are relying on
reliable try results, all this has completely changed. These are the
new rules:
- There is now a dedicated sheriffing service, that means, patches
are landed at defined times. - Test failures will not be tolerated, anything that breaks a test
will be backed out not to affect others. - Most people, including those with Level 3 rights, set "checkin
needed" on the bug and the sheriff will take care of landing the
patch. Mostly I land patches after the following M-C merge times
(all GMT): 10 AM, 4 PM and 10 PM, Geoff lands after the 4 AM merge. - Of course developers with Level 3 rights can still land their own
patches, especially large patches or multi-part ones, but it's
appreciated to supply patches to the "checkin needed" pool for use
at the specified times.
Some other measures of interest:
- Failing tests which can't be fixed via a backout will be disabled
and marked especially in BMO. - Any other bustage, like compile or build bustage, will usually be
fixed within a few hours after occurring, typically with patches
that get the so-called PLR (post landing review).
All this is done to enable others to build and run successful try runs
at any time. The new rules have been in place since 2017 and have
worked very well.
Jörg.
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net
On 10/9/19 7:09 PM, Jörg Knobloch wrote:
Hi all,
as you know, some of us have Level 1 access rights, that is, they can
push to try repositories, and others have Level 3 access rights, they
can push to (mostly) all repositories, including comm-central,
comm-beta, comm-esr68, and even mozilla-central (don't! - you get into
trouble).
Traditionally people have pushed their own patches to comm-central,
but this practise has mostly stopped now for various reasons.
In the olden days, people just pushed stuff and then walked away as
the tree was mostly completely orange anyway. Back then, Aleth came
around and slapped people on the fingers when they landed bustage ;-)
- Landings weren't coordinated with M-C merges, so it was hard to find
regressions since between two C-C pushes there could have been any
number of M-C merges. The tree was frequently closed for days or even
weeks due to bustage.
As there are many people working on C-C code now who are relying on
reliable try results, all this has completely changed. These are the
new rules:
- There is now a dedicated sheriffing service, that means, patches
are landed at defined times. - Test failures will not be tolerated, anything that breaks a test
will be backed out not to affect others. - Most people, including those with Level 3 rights, set "checkin
needed" on the bug and the sheriff will take care of landing the
patch. Mostly I land patches after the following M-C merge times
(all GMT): 10 AM, 4 PM and 10 PM, Geoff lands after the 4 AM merge. - Of course developers with Level 3 rights can still land their own
patches, especially large patches or multi-part ones, but it's
appreciated to supply patches to the "checkin needed" pool for use
at the specified times.
Some other measures of interest:
- Failing tests which can't be fixed via a backout will be disabled
and marked especially in BMO. - Any other bustage, like compile or build bustage, will usually be
fixed within a few hours after occurring, typically with patches
that get the so-called PLR (post landing review).
All this is done to enable others to build and run successful try runs
at any time. The new rules have been in place since 2017 and have
worked very well.
Jörg.
Thanks for writing this out in response to my question! I hadn't
realized there was any changes to the "normal" way we've done things. Is
this documented in our current developer docs?
Thanks for sheriff-ing! 👮
--Patrick
On 10.10.19 01:09, Jörg Knobloch wrote:
In the olden days, people just pushed stuff and then walked away as the tree was mostly completely orange anyway. Back then, Aleth came around and slapped people on the fingers when they landed bustage ;-) - Landings weren't coordinated with M-C merges, so it was hard to find regressions since between two C-C pushes there could have been any number of M-C merges. The tree was frequently closed for days or even weeks due to bustage.
I think this should be fixed on a technical level:
- Pushes to m-c (which are batched nowadays anyway) should trigger rebuilds and tests. This means that failures caused by m-c are automatically and correctly blamed.
- All pushes to c-c should trigger rebuilds, to see what broke things.
- We either:
- ask developers to stick around and fix oranges, or
- we make a separate push branch that gets built on push, and on success, automatically gets merged to c-c, and the developer gets emailed on the result either way.
The above of course presumes that tests are somewhat reliable and not completely random. Unreliable tests (also called "randomly passing tests") need to be systematically disabled or fixed.
Thanks to Jörg for keeping the tree green (or as green as possible)! We've said it often, but Jörg is fighting every day, so he deserves the thanks.
Ben