Closed
Bug 1055524
Opened 10 years ago
Closed 10 years ago
Dashboard check automatically managed by ship-it
Categories
(Release Engineering :: Release Automation: Other, defect)
Release Engineering
Release Automation: Other
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Sylvestre, Assigned: Sylvestre)
Details
(Whiteboard: [shipit])
Attachments
(1 file, 1 obsolete file)
11.05 KB,
patch
|
bhearsum
:
review+
|
Details | Diff | Splinter Review |
The proposed patch managed the dashboard check transparently. It is a beta => enable it It is a release (or other cases) => disable it
Attachment #8475142 -
Flags: review?(rail)
Attachment #8475142 -
Flags: review?(bhearsum)
Comment 1•10 years ago
|
||
Comment on attachment 8475142 [details] [diff] [review] 0001-Dashboard-check-automatically-managed.patch Review of attachment 8475142 [details] [diff] [review]: ----------------------------------------------------------------- I'm really unhappy that this patch needs to exist at all. The only reason this option exists is because the l10n dashboard doesn't support the use cases we need it to. I'd much rather effort be spent fixing it than working around it even more. ::: kickoff/model.py @@ +12,5 @@ > > +def isBeta(version): > + if re.match('.*b[0-9]', version): > + return True > + return False I would prefer to see ANY_VERSION_REGEX being used here rather than a custom regex. It's a much more well tested and used regex. You can check its groups to detect a beta: >>> from mozilla.build.versions import ANY_VERSION_REGEX >>> import re >>> a = re.match(ANY_VERSION_REGEX, "32.0b8") >>> a <_sre.SRE_Match object at 0x7ff657c6add8> >>> a.groups() ('32.0', 'b8', 'b', None, None) ::: kickoff/templates/includes/fennec_release.html @@ -50,5 @@ > -<div class="submit_release"> > - {{ fennecForm.dashboardCheck.label()|safe }} > - {{ fennecForm.dashboardCheck()|safe }} > - <div class="help">Whether or not to ensure that the changesets given match the ones on the <a href="https://l10n.mozilla.org/shipping/milestones">L10n Dashboard</a>. This is normally turned on for betas and turned off for all other releases.</div> > -</div> I think it's a bad idea to remove this override. If something comes up where the the l10n dashboard is broken, you won't be able to kick off betas without releng manual work. I would suggest moving this to the Advanced Options section instead.
Assignee | ||
Comment 2•10 years ago
|
||
Thanks for the feedback. After chatting on IRC with Ben: A better plan would be to move it to advanced, document it a bit more and update the javascript to check the box when we deal with a beta.
Updated•10 years ago
|
Attachment #8475142 -
Attachment is obsolete: true
Attachment #8475142 -
Flags: review?(rail)
Attachment #8475142 -
Flags: review?(bhearsum)
Assignee | ||
Comment 3•10 years ago
|
||
Better management of the dashboard check: * Moved to advanced options * When building a beta release, tick the checkbox (not in other cases) * Improve a little the documentation I took the liberty to do a minor refactoring in the same patch.
Attachment #8475739 -
Flags: review?(bhearsum)
Updated•10 years ago
|
Attachment #8475739 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Thanks. Pushed: http://git.mozilla.org/?p=build/release-kickoff.git;a=commit;h=1cec9deec611df9e6ea239ced93b8e6fdb532f4a
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → sledru
Assignee | ||
Comment 5•10 years ago
|
||
Deployed and working.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•