1392488 - Title of email for try push should ignore commit of try fuzzy

Status: RESOLVED INCOMPLETE

(Release Engineering :: General, enhancement)

Description

[Xidorn Quan [:xidorn] UTC+11]

When a try push is done with traditional try syntax, the email notification sent for the try push would be the commit message before the try syntax commit.

However, with try fuzzy, the emails are now all titled
> Pushed via 'mach try fuzzy', see diff for scheduled tasks
which is not quite useful when browsing email list.

Comment 1

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

I think that's controlled in https://hg.mozilla.org/build/buildbotcustom/file/tip/status/generators.py#l47

Comment 2

[Andrew Halberstadt [:ahal]]

I have a patch that's going to change this to include the query used to something like:
Pushed via 'mach try fuzzy' with query: 'stylo

With that change would you be ok WONTFIXING this?

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

No, that doesn't help. The commit message is still much more useful than what task am I running. Majority of people usually run the same set of tasks in most of time, so what is in the query doesn't really provide much information.

Comment 4

[Andrew Halberstadt [:ahal]]

Ah, I misunderstood what you meant. Yeah this bug makes sense, thanks for filing! (I never noticed as I redirect try e-mails to trash).

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1392488 - Skip commit created by try fuzzy for sensible commit title.

Review commit: https://reviewboard.mozilla.org/r/175836/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175836/

Comment 6

[Chris AtLee [:catlee]]

Comment on attachment 8904085 [details]
Bug 1392488 - Skip commit created by try fuzzy for sensible commit title.

https://reviewboard.mozilla.org/r/175836/#review181304

::: status/generators.py:54
(Diff revision 1)
>      """
>      Returns the first non-trychooser title with unnecessary cruft removed.
>      """
>      for title in titles:
> +        # Skip try fuzzy commit.
> +        if title.startswith('Pushed via '):

can we make this more specific to the try fuzzy commit message?

Comment 7

[Andrew Halberstadt [:ahal]]

Just in case this informs the approach taken here, in the future there will be more than one try_task_config.json based try selector, e.g we might have: ./mach try path

It might be a good idea to standardize on some sort of token in the commit message for all of these. I don't know if we want to make it 'Pushed via' or not though. This could also be left for follow-up when we actually do have more than one selector.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8904085 [details]
Bug 1392488 - Skip commit created by try fuzzy for sensible commit title.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175836/diff/1-2/

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

Patch updated.

Comment 10

[Chris AtLee [:catlee]]

Comment on attachment 8904085 [details]
Bug 1392488 - Skip commit created by try fuzzy for sensible commit title.

https://reviewboard.mozilla.org/r/175836/#review183292

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

It seems I don't have permission to checkin code into this repo.

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

Chris, could you help checking this in?

Comment 13

[Chris AtLee [:catlee]]

Sure thing.

I checked it in here: https://hg.mozilla.org/build/buildbotcustom/rev/45e90cfbfd5e

It should be deployed later in the week.

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

Thanks.

Comment 15

[Andrew Halberstadt [:ahal]]

This is going to be regressed by bug 1400429 which moves the "Pushed via" to the second line (since this patch uses startswith). Note the quote is also being changed to a backtick. Also, there will now be:

Pushed via `mach try syntax`
Pushed via `mach try empty`

messages that we also want to catch. So I'd suggest not making this 'fuzzy' specific. Maybe we could change the condition to:

if "Pushed via `mach" in line:

Comment 16

[Andrew Halberstadt [:ahal]]

Actually, it looks like "titles" only contains the first line of the commit message, in which case we can't use this to detect the "Pushed via" anymore since it's no longer on the first line.

Also worth noting this would be broken by bug 1400469 which will allow arbitrary descriptions in a |mach try| push.

Comment 17

[Julien Cristau [:jcristau]]

The email no longer includes commit descriptions.