1297011 - Invalid versions should not fetch l10n changesets

Status: RESOLVED FIXED

(Release Engineering :: Applications: Shipit, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 1

[Rail Aliiev [:rail]]

Johan, can you look at this?

Comment 2

[Johan Lorenzo [:jlorenzo]]

Sure!

Comment 3

[Johan Lorenzo [:jlorenzo]]

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

Comment 4

[Johan Lorenzo [:jlorenzo]]

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.

Comment 5

[Johan Lorenzo [:jlorenzo]]

Attachment: 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?

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

Deployed PR #114 as part of a push for another change.

Comment 7

[Johan Lorenzo [:jlorenzo]]

Landed at https://github.com/mozilla-releng/ship-it/commit/80235e2312a621504a2c9a3343893fda6eb6c589