1120475 - Hook to display a message during merge weekends

Status: NEW

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

Description

[Nigel Babu [:nigelb]]

During merge weekends, we have a lot of pushes over the weekend which are landed half baked and in a hurry with the hopes of catching the train. This leads to a mess left for Monday morning.

Callek suggested we put a message that shows up after the push warning folks that they don't need to crash land and there is an approval process.

This is mostly a discussion bug for now. If we decide to implement, I'm happy to do the work if there's nobody else with capacity.

Things we'll need to resolve: How will the hook know that it's merge weekend? hwine has made it clear that manual is a non-starter.

Comment 1

[hwine]

adding rail - the merge scripts might be a place to manipulate the "is it merge day yet?"

Comment 2

[Ed Morley [:emorley]]

We could add an additional tree state alongside {open,closed,approval required} in treestatus.m.o, which could feed the hook.

Comment 3

[Ed Morley [:emorley]]

Perhaps a more generic way of fixing this would be to:
* Add a new state "warning" to treestatus:
https://github.com/mozilla/treestatus/search?utf8=%E2%9C%93&q=approval&type=Code
* modify the existing tree closure hghook, so that if the state was 'warning', we output the 'reason' field in the response, like we do for closed/approval required already:
https://hg.mozilla.org/hgcustom/version-control-tools/file/8c36430488c8/hghooks/mozhghooks/treeclosure.py#l40

Comment 4

[Ed Morley [:emorley]]

Or we just keep on using 'open' (since with 'warning', the intent would be for the tree to still be open) and just output the message if 'reason' is not the empty string.

Comment 5

[Gregory Szorc [:gps]]

We already query treestatus as part of pushes. I'll stand pretty firm about not introducing more external dependencies during pushes (it's an avoidable source of latency and operational dependency). Let's figure out a way to integrate this into the existing treestatus API response.

Comment 6

[Nigel Babu [:nigelb]]

Shall we just look for "merge" in "message_of_the_day"? That seems simple enough to check and easy enough to control for the sheriffs plus doesn't require a code change on treestatus.

Comment 7

[Rail Aliiev [:rail]]

I'd say let's WONTFIX this. No one reads the messages unless the push exits non zero.

Comment 8

[Phil Ringnalda (:philor)]

Actually they don't necessarily read them even then, I've seen a fair number of people realize hours later that they bounced off a closed tree hook without noticing because they didn't look at all, just pushed and went home, like we've spent years persuading them that they should.

Instead of going to a lot of work on this technical solution to a social problem, why not try this? After the next merge weekend, just ask all the people who pushed broken crap whether or not they knew that they could request approval-aurora? 100% of them will say "yes, I know, but I didn't want to," at which point you'll know that this hook will do absolutely no good whatsoever.

Comment 9

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

Attachment: MozReview Request: bz://1120475/KWierso

Comment 10

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

/r/2583 - Bug 1120475 - Enhance the treeclosure hook to warn users when they push on merge weekend.

Pull down this commit:

hg pull review -r c4e66a9308b8b1b695cb876b947852b9823fe36f

Comment 11

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

(In reply to Wes Kocher (:KWierso) from comment #9)
> Created attachment 8550151 [details]
> MozReview Request: bz://1120475/KWierso

Don't really care whether this gets wontfixed, but thought I'd put this up as half of it was written before I read rail/philor's comments.

I figure it wouldn't hurt to at least show the message to people when they push, whether they disregard it or not.

Comment 13

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

Attachment: MozReview Request: Bug 1120475 - Enhance the treeclosure hook to warn users when they push on merge weekend.