[shipping] add counts to the locale-changes page to indicate current, recent, and old locales

RESOLVED FIXED

Status

Webtools
Elmo
P2
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Pike, Assigned: peterbe)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
We'd like to understand better how many locales are good and how many are lagging.

To make that easier, we'd rather not count ourselves but have elmo do that.

First bucket is current, second number is on the previous release, third bucket is "older than that".

Mockup could be like:

20 | ab-CD    | 1

And then a row group after that for the old bucket and the current bucket (which might span multiple beta milestones)

Giving this a P2, as it'd be nice to have that soon to drill down our goals for next quarter.
(Assignee)

Comment 1

5 years ago
Pardon my goldfish-like memory, but what's the "locale-changes page"?

I wouldn't mind working on this but I think I'd need a refresher about what it is and how it would work.
(Reporter)

Comment 2

5 years ago
https://l10n.mozilla.org/shipping/app/locale-changes/fx25 is an example URL, it's in apps/shipping/views/app.py
(Assignee)

Comment 3

5 years ago
Wow! I started reading the code and my head exploded. In a bad way :)
I think we better step through this bug in the next Elmo call. Then you can explain how you want this to work.
(Assignee)

Updated

5 years ago
Assignee: nobody → peterbe
(Assignee)

Comment 4

5 years ago
Created attachment 8338095 [details]
Screen Shot 2013-11-25 at 3.24.05 PM.png

The code I had to write to get to this is pretty horrible so I won't share yet but looking at the output (and pausing on aesthetics for the moment too) is this what you'd expect?

I reversed the list of rows, and put things in buckets if the code name was the same (this puts fx23_beta_1 together with fx23)

Then as I loop through all the rows, just before I dump a lump into a bucket I calculate the rowspan (and count the number of unique locales) and put that into the "last" row so it becomes easy to render in the template. 

What stumped me big time was that you said you wanted 4 groups and the third group should be fx20 and fx21. That I have not been able to achieve. I don't think I understand the data well enough to be able to make those groups.
Attachment #8338095 - Flags: feedback?(l10n)
(Assignee)

Comment 5

4 years ago
Created attachment 8341959 [details]
Screenshot 2013-12-03 16.43.45.png

This way, the last of the 4 groups' rowspan is top vertical aligned. 

Now I just need to write a unit test to back this all up.

Comment 6

4 years ago
screenshot looks like what we need.
(Assignee)

Comment 7

4 years ago
Created attachment 8342475 [details] [diff] [review]
bug853068-2.diff

I tried to start writing a test to prove this but there's nothing to build on for a test. I got started but quickly realised I would have to make fixture for every model under the sun. It just got messier and messier so in favor of progress some time this year I chickened out on a working test. 

The code is still a bit of a blur to me how it works with all the optimized dicts and lists of IDs. Perhaps it could be a follow-up bug to rewrite this into something more structured and modular.
Attachment #8342475 - Flags: review?(l10n)
(Reporter)

Comment 8

4 years ago
Comment on attachment 8342475 [details] [diff] [review]
bug853068-2.diff

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

I don't think this actually does what we talked about.

The groups were:

This cycle
Last cycle
Prior two cycles
Old cycles

I find the code hard to grok as well, it'd be great if there were more comments on what that code actually does.

Lastly, I'd prefer to go for classes for the rowspan rather than inline style, where possible.

I'm OK with chickening out of a test for this, though.
Attachment #8342475 - Flags: review?(l10n) → review-
(Reporter)

Comment 9

4 years ago
Comment on attachment 8338095 [details]
Screen Shot 2013-11-25 at 3.24.05 PM.png

This one showed the right numbers, as there's no Firefox 20.

The only request I had for this was to align the last bucket top.
Attachment #8338095 - Flags: feedback?(l10n) → feedback+
(Assignee)

Comment 10

4 years ago
The code under review lumps (aka. buckets) like this (from the bottom):

1. Latest release
2. Second latest release
3. Third latest release
4. All other older releases

I believe you said you wanted it as:

1. Latest release
2. Second latest release
3. Third AND forth latest release
4. All other older releases

This was something I could NOT make happen with the code. If you can please have a go :)
Flags: needinfo?(l10n)
(Reporter)

Comment 11

4 years ago
I think if you hook up the group logic inside the loop in line 58

    while av_ and locs4av:

it should be OK to get the same offset in releases grouped together. Probably keeping a counter there, storing the offset in rows, and inc'ing by len(accepted4loc), and on the next group bump, add the extra data to the row you kept. You can also ignore dealing with the milestone rows.

There's a bit of paperwork, but I don't think it'll be worse than the code you're having.
Flags: needinfo?(l10n)
(Reporter)

Comment 12

4 years ago
I've taken this branch and rewrote the logic to support variable-sized buckets.

The result is in https://github.com/Pike/elmo/compare/mozilla:develop...feature%2Fbug-853068-group-locale-changes?expand=1.

There's one caveat, Peter, can you look at that? I can't figure out why the rowspan="2" doesn't open up the last column, there's still a full-width border between the two rows. The larger first and last buckets work fine and block out the border from the last column.
(Assignee)

Comment 13

4 years ago
I tested it locally. It seems to work like a charm! You're good at this kind of stuff. 

How do you want to proceed? That you make a patch for final review or that I take your fork and make a patch for final review? After all, now it's almost entirely your code.
(Reporter)

Comment 14

4 years ago
I'd appreciated if you could figure out the remaining html table problem here.
(Reporter)

Comment 15

4 years ago
Created attachment 8374957 [details]
show the table color, red square content bothers me

Here's a screenshot to show where the table border is that bothers me, I squared it in red. The 8 actually counts 'nso te' and 'af bn-BD fa kk mk vi', aka, 18 and 19. But there's a border that looks like it's only counting 19.
(Assignee)

Comment 16

4 years ago
Created attachment 8375037 [details] [diff] [review]
bug853068-3.diff

Your work, my css hack :)
Attachment #8342475 - Attachment is obsolete: true
Attachment #8375037 - Flags: review?(l10n)
(Reporter)

Updated

4 years ago
Attachment #8375037 - Flags: review?(l10n) → review+

Comment 17

4 years ago
Commit pushed to develop at https://github.com/mozilla/elmo

https://github.com/mozilla/elmo/commit/90de3e64708a7058e958bd0ef4924381e5c20c56
bug 853068 - add counts to the locale-changes page to indicate current, recent, and old locales, r=Pike
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.