1515009 - Deduplicate code for displaying multi-store data in about:telemetry

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P3)

Description

[Jan-Erik Rediger [:janerik]]

Bug 1498169 added displaying multi-store data.
That lead to a bit of code duplication due to handling the multi-store and non-multi-store case.

This can probably be simplified a bit.

Comment 1

[l.khadka]

I would like to work on this

Comment 2

[Jan-Erik Rediger [:janerik]]

I assigned the bug to you. Do you need more info from me of what exactly needs to be done?

Comment 3

[l.khadka]

Yes, I'd really like to get some help with this.

Comment 4

[Jan-Erik Rediger [:janerik]]

First, sorry for taking longer than necessary to write down some guidance.

I assume you already have a Firefox build set up. If not download and build the Firefox source code.

Going through the code again just minor duplications are left, but still worthwhile to tackle.
The following are pieces that should be deduplicated:

• These loops and the const are essentially the same.
  The code can be moved into a function and be called in both places.
• The headings const is used in both code paths.
  It can be unified and moved to above the if branching (line 1681)

If you find any other obvious code duplication while reading the code, feel free to work on that as well.
Don't bother with simple 1-2 lines of duplication or code that would require too much shuffling around.

You can test any changes by building Firefox using mach build, then running it with mach run and browsing to about:telemetry.
There check that histograms, keyed histograms, scalars and keyed scalars are displayed correctly and that in the left menu you see the main store.

Once you're done submit the patch for review. Mark me as a reviewer so I'll get an email to come look at your code.
See the guide on how to Submit a Patch if you haven't done that before.

Don't hesitate to ask if there's anything unclear.

Comment 5

[l.khadka]

Attachment: Bug 1515009 Deduplicate code for displaying multi-store data in about:telemetry

Comment 6

[l.khadka]

Sorry for taking so long with the patch. Could you please review it?

Comment 7

[l.khadka]

Attachment: Bug 1515009 add keyedScalars as a parameter to fix eslint error

Depends on D20083

Comment 8

[l.khadka]

Attachment: Bug 1515009 Pass both scalar name and scalar data instead of full scalar object

Depends on D20092

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1044ef0e7225
Deduplicate code for displaying multi-store data in about:telemetry r=janerik

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1044ef0e7225

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1044ef0e7225