Invalid versions should not fetch l10n changesets

RESOLVED FIXED

Status

Release Engineering
Ship It
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: sylvestre, Assigned: jlorenzo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

PR
50 bytes, text/x-github-pull-request
rail
: review+
Details | Review | Splinter Review
(Reporter)

Description

a year ago
Enter 23.0, this is a valid version, I get a proper list of changeset.
Now, enter 23.0b2.0, obvious, ship-it cannot retrieve the list of changeset as the version doesn't exist.
However, I still get the list of changeset for 23.0.

So, when we touch the version field, we should automatically reset the l10n changeset.
Johan, can you look at this?
Flags: needinfo?(jlorenzo)
(Assignee)

Comment 2

a year ago
Sure!
Assignee: nobody → jlorenzo
Status: NEW → ASSIGNED
Flags: needinfo?(jlorenzo)
(Assignee)

Comment 3

a year ago
I'm sorry, I'm not sure to understand the issue. Should 23.0b2.0 be considered as an invalid version, so we shouldn't retrieve l10n changesets?

In the current state, 23.0b2.0 does reset the l10n field[1]. But it goes fetch https://l10n.mozilla.org/shipping/l10n-changesets?av=fx23 , and as the result is already cached, the field is filled nearly instantaneously. That's why, it seems like the there was no reset done. 

[1] https://github.com/mozilla-releng/ship-it/pull/103/files#diff-f2f19f23c632c1a8154fd734bebd6ed1R238
Depends on: 1292188
Flags: needinfo?(sledru)
(Assignee)

Comment 4

a year ago
From Sylvestre on IRC:
> 10:03:35 <Sylvestre> 23.0b2.0 is invalid, so, we should not have anything in the l10n field 

Changing the title accordingly.
Flags: needinfo?(sledru)
Summary: Changing the version should reset the l10n changeset field → Invalid versions should not fetch l10n changesets
(Assignee)

Comment 5

a year ago
Created attachment 8783672 [details] [review]
PR

This patch makes the JS-side behaving similarly to the Python one (the one defined in bug 1296168). The regexp that says if a version number is valid is the same in both cases.

There is now room for a few enhancements:
* Centralize the regex in 1 file, and make both Python and JS read from it. 
* Split release.js into version.js and release.js. Most of what's in release.js is actually making sure we have a valid version number. The only part specific to a release is the build number
* Once version.js is defined: Use it everywhere we can in suggestions.js. This will make functions like isBeta(version) useless.
* Show a warning when a bad version number is entered.

I propose to make these changes in follow ups. Changing the regex on the JS-side led to a considerable amount of changes already.

What do you think, rail?
Attachment #8783672 - Flags: review?(rail)
(Assignee)

Updated

a year ago
Depends on: 1296168
(Assignee)

Updated

a year ago
Blocks: 1297483
Attachment #8783672 - Flags: review?(rail) → review+
Deployed PR #114 as part of a push for another change.
(Assignee)

Comment 7

a year ago
Landed at https://github.com/mozilla-releng/ship-it/commit/80235e2312a621504a2c9a3343893fda6eb6c589
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.