1537890 - Perfherder shouldn't center numbers in their cells and should use thousands separators for formatting them

Status: RESOLVED FIXED

(Tree Management :: Perfherder, enhancement, P3)

Description

[:Gijs (he/him)]

cf.

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=autoland&originalRevision=85bd45178007470651a23a54a416ac9208b1d531&newProject=autoland&newRevision=4f6d8ed9e94809f911907725a708627e3faaed01&originalSignature=1885901&newSignature=1885901&framework=4

All of these numbers (besides the margin of error) are unfit for human consumption.

Comment 1

[Ionuț Goldan [:igoldan]]

Sarah, I think this bug was already fixed. Or am I wrong?

Comment 2

[Sarah Clements [:sclements]]

No this still needs to get done. It's an easy fix I'll get pushed out soon.

Comment 3

[Sarah Clements [:sclements]]

Actually this will make a good first bug for prospective Outreachy applicants so I'll un-assign it.

Comment 4

[Viktor Stojanov [:Draci]]

Hi, I am an Outreachy applicant and I would like to contribute by fixing this bug.
Thank you!

Comment 5

[Sarah Clements [:sclements]]

Hi Viktor, I'll assign it you.

Comment 6

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/5421

Comment 7

[Ionuț Goldan [:igoldan]]

I don't think this ticket is done yet. According to the summary, we were required to provide the thousands separator to all our numbers from Compare view, but we're still lacking this.

Comment 8

[:Gijs (he/him)]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriff from comment #7)

I don't think this ticket is done yet. According to the summary, we were required to provide the thousands separator to all our numbers from Compare view, but we're still lacking this.

TBF, I should probably have filed a separate bug for the thousands separator / number formatting... we could clone this bug and provide an opportunity for Draci or another outreachy applicant to fix that separately.

Comment 9

[Viktor Stojanov [:Draci]]

I would gladly finish whatever I missed on the first run. After closer inspection of the link provided the only place I can see that is missing the thousands separators is on hover. Is that what you're referring to @Ionuț ? If @Sarah is OK with it I would like to do what @Gijs mentioned above.

Comment 10

[Sarah Clements [:sclements]]

Attachment: Screen Shot 2019-10-08 at 10.24.12 AM.png

It looks like we missed the numbers in the tooltip. I'll reopen this bug and :Draci can do a follow up pr (Draci just use this same bug number and include a description of the commit that this is for formatting numbers in tooltip).

Comment 11

[Sarah Clements [:sclements]]

Viktor, were you still planning to complete a follow up pull request for this bug?

Comment 12

[Viktor Stojanov [:Draci]]

Hi :sclements,

I do plan on finishing this. I will finish this and the second part this week.

Thank you for your patience!

Comment 13

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/5551