1256998 - Include links to the l10n changesets in the build email

Status: RESOLVED FIXED

(Release Engineering :: Release Automation, defect)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

In the email sent to release-drivers when we start a build, we could include a link to the associated l10n changeset. (This would be for releng to use until some better solution is found )  

For example, for 46 beta 2 desktop include a link to https://ship-it.mozilla.org/releases/Firefox-46.0b2-build2/l10n   and for 46 beta 2 fennec include one to https://ship-it.mozilla.org/releases/Fennec-46.0b2-build1/l10n .

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

The point here is also for releng to not have to login to ship-it to find these changesets for build promotion.

Comment 2

[Johan Lorenzo [:jlorenzo]]

Attachment: Bug 1256998 - Include links to the l10n changesets in the build email

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

Comment 3

[Johan Lorenzo [:jlorenzo]]

I made an educated guess about the variable to use, but I haven't been able to test it (even manually). I looked for hints (like in bug 1258568), but I didn't manage to find some. Before asking for a review, what's the best way to test email_release_drivers()?

Comment 4

[Rail Aliiev [:rail]]

buildbot-master83 runs a staging instance of release runner, which talks to https://ship-it-dev.allizom.org/

Emails are send to the release-notifications-dev ML. You would need to apply your changes there, restart release runner and create a new release based on jamun (http://hg.mozilla.org/projects/jamun)

Comment 5

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

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

Comment 6

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

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

Comment 7

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72276/diff/3-4/

Comment 8

[Johan Lorenzo [:jlorenzo]]

Unbitrotten. Latest revision of the patch was tested in: https://groups.google.com/a/mozilla.com/forum/#!msg/release-automation-notifications-dev/yqmcu-4ZKC0/w-ndVLhWAgAJ

Comment 9

[Jordan Lund (:jlund)]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

https://reviewboard.mozilla.org/r/72276/#review76372

looks great. one issue and one nit that you can ignore if you like :)

r- for now until the open issue is patched or commented on

::: lib/python/kickoff/__init__.py:107
(Diff revision 4)
>      content = """\
>  A new build has been submitted through ship-it:
>  
>  Commit: https://hg.mozilla.org/{path}/rev/{revision}
>  Task group: https://tools.taskcluster.net/push-inspector/#/{task_group_id}/
> +L10n changesets:  {l10n_url} (requires VPN access)

through some markup magic, https://groups.google.com/a/mozilla.com/forum/#!msg/release-automation-notifications-dev/yqmcu-4ZKC0/w-ndVLhWAgAJ seems to think that the push inspector url is:

https://tools.taskcluster.net/push-inspector/#/s4Man_AxTTyg4D-0K9AMig/L10n

I wonder if trimming the trailing '/' from push-inspect url on previous line will fix that..

::: lib/python/kickoff/api.py:41
(Diff revision 4)
>          self.raise_exceptions = raise_exceptions
>          self.session = requests.session()
>          self.csrf_token = None
>          self.retries = retry_attempts
>  
>      def request(self, params=None, data=None, method='GET', url_template_vars={}):

not related to your diff but maybe we should:

s/url_template_vars={}/url_template_vars=None/

in the meth signature and then in the body, initialize it to {} if None

Comment 10

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72276/diff/4-5/

Comment 11

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72276/diff/5-6/

Comment 12

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

It turns out removing the trailing slash doesn't fix the URL. I guess "L10n" is detected as a non-common word by Google. I propose to change the wording into "Locales".

I'll test that word out, once bug 1301380 is fixed. Otherwise people will get notifications in the production channel.

Comment 13

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

"Locales" instead of "L10n" works better[1]!

[1] https://groups.google.com/a/mozilla.com/forum/#!msg/release-automation-notifications-dev/R5gY9uv05yM/ZFiirbvnAQAJ

Comment 14

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/72276/diff/6-7/

Comment 15

[Jordan Lund (:jlund)]

Comment on attachment 8781977 [details]
Bug 1256998 - Include links to the l10n changesets in the build email

https://reviewboard.mozilla.org/r/72276/#review77324

lgtm :)

Comment 16

[Johan Lorenzo [:jlorenzo]]

https://hg.mozilla.org/build/tools/rev/cab42e42703aed9bf67e08ca9dd8dcf8ed12955c
Bug 1256998 - Include links to the l10n changesets in the build email r=jlund

Comment 17

[Johan Lorenzo [:jlorenzo]]

Pushed and deployed to bm8{3,5}