1074228 - Build start email should differentiate the person who created it and the person who started it

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

Given my timezone, it is common that I prepare the ship-it build and I rely on Lukas, Lawrence or Bhavana to start the build for me once the tree are green.

In the email, only the person who created the job is listed. In case author != starter, the second should be displayed in the email.

Example:
---

A new build has been submitted through ship-it:
mozilla commit: https://hg.mozilla.org/releases/mozilla-release/rev/f9ccc64ca17d

[e-mail]
Build started by [e-mail]
---

This might require a new field in the database.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: 0001-Bug-1074228-Store-the-name-of-the-person-who-is-star.patch

Add a new field in the database to store who is starting the build.

It is now common that a release manager in Europe creates the job and an other release manager (let's say in Toronto or San Francisco) starts the build once the trees are green.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: 0001-Bug-1074228-If-the-person-who-started-the-build-is-d.patch

Update release runner to send the info in the email.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

I will need your help to put that in production since the database needs to be updated.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 8518233 [details] [diff] [review]
0001-Bug-1074228-Store-the-name-of-the-person-who-is-star.patch

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

::: kickoff/views/releases.py
@@ +133,5 @@
>              r.comment = form.comment.data
> +            if r.submitter != starter:
> +                # A different person created the build and started it
> +                # We want to store this information for the email
> +                r.starter = starter

Why do we only want to store the starter if they're different than the submitter? Shouldn't we be storing this every time?

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 8518235 [details] [diff] [review]
0001-Bug-1074228-If-the-person-who-started-the-build-is-d.patch

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

::: buildfarm/release/release-runner.py
@@ +231,5 @@
>      contentMail += "\n" + r["submitter"] + "\n"
>  
> +    if r["starter"] and r["submitter"] != r["starter"]:
> +        # The person who is starting the build is not the same who created
> +        contentMail += "\nStarted by " + r["starter"] + "\n"

I think we should show this all the time, too. It seems like it would be confusing to only have "Started by" in some of the e-mails.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

(In reply to Ben Hearsum [:bhearsum] from comment #4)
> Why do we only want to store the starter if they're different than the
> submitter? Shouldn't we be storing this every time?
Why not. I don't mind.
I considered that it is interesting to store it only if it is different.
Let me know what you prefer!

Comment 7

[Sylvestre Ledru [:Sylvestre]]

(In reply to Ben Hearsum [:bhearsum] from comment #5)
> I think we should show this all the time, too. It seems like it would be
> confusing to only have "Started by" in some of the e-mails.
In most of the cases (when Lukas or Lawrence are managing beta, or half of the time for me), we will have:
Created by foo
Started by foo

Is it really interesting in this case?

Comment 8

[[e-mail] (:bhearsum)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #7)
> (In reply to Ben Hearsum [:bhearsum] from comment #5)
> > I think we should show this all the time, too. It seems like it would be
> > confusing to only have "Started by" in some of the e-mails.
> In most of the cases (when Lukas or Lawrence are managing beta, or half of
> the time for me), we will have:
> Created by foo
> Started by foo
> 
> Is it really interesting in this case?

It's not that it's interesting, it's that not having it is confusing. If some e-mails say "Started by" I start to get the expectation that they will tell me who started a release. When I see other e-mails that don't tell me explicitly, I could get confused. I think we may as well do this since it's only a single extra line in the e-mail.

(In reply to Sylvestre Ledru [:sylvestre] from comment #6)
> (In reply to Ben Hearsum [:bhearsum] from comment #4)
> > Why do we only want to store the starter if they're different than the
> > submitter? Shouldn't we be storing this every time?
> Why not. I don't mind.
> I considered that it is interesting to store it only if it is different.
> Let me know what you prefer!

Yeah, let's always store it. Otherwise we're making an assumption now (creator == starter if starter isn't provided) that may not hold true in the future.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

OK, thanks for the feedback. I will update my patch with these two changes

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Attachment: 0001-Bug-1074228-Store-the-name-of-the-person-who-is-star.patch

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Attachment: 0001-Bug-1074228-If-the-person-who-started-the-build-is-d.patch

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 8520473 [details] [diff] [review]
0001-Bug-1074228-If-the-person-who-started-the-build-is-d.patch

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

Make sure you don't land this one until the ship it patch is in production.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8520472 [details] [diff] [review]
0001-Bug-1074228-Store-the-name-of-the-person-who-is-star.patch

merged
http://git.mozilla.org/?p=build/release-kickoff.git;a=commitdiff;h=ef28f02d85af16f8b572dd8595c72f1f5c0fc826;hp=c3f1281895a63b07c1667230a16c93929614df05

Comment 14

[[e-mail] (:bhearsum)]

This was pushed to production today, including the necessary database upgrade.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8520473 [details] [diff] [review]
0001-Bug-1074228-If-the-person-who-started-the-build-is-d.patch

https://hg.mozilla.org/build/tools/rev/4215f55209aa

Comment 16

[Sylvestre Ledru [:Sylvestre]]

That works (cf Build of Fennec-34.0b8-build3)
---
Created by [e-mail]

Started by [e-mail]
---