1513546 - Warning for GetKeyedScalar for CHILD says "GetScalarByEnum" not "GetKeyedScalarByEnum"

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P3)

Description

[Chris H-C :chutten]

The warning message here [1] should say "NS_FAILED internal_GetKeyedScalarByEnum for CHILD" instead of "NS_FAILED internal_GetScalarByEnum for CHILD".

[1]: https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/toolkit/components/telemetry/core/TelemetryScalar.cpp#2272

Comment 1

[Alessio Placitelli [:Dexter]]

Hey, do you want to make this mentored?

Comment 2

[Chris H-C :chutten]

To help Mozilla out with this bug, here's the steps:

0) Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
1) Download and build the Firefox source code: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build
- If you have any problems, please ask on IRC (https://wiki.mozilla.org/Irc) in the #introduction channel. They're there to help you get started.
- You can also read the Developer Guide, which has answers to most development questions: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction
2) Start working on this bug. You'll be working in the TelemetrScalar.cpp file, rewriting this warning [1] so that it says "NS_FAILED internal_GetKeyedScalarByEnum for CHILD". (notice the "Keyed")
- If you have any problems with this bug, please comment on this bug and set the needinfo flag for me. Also, you can find me and my teammates on the #telemetry channel on IRC (https://wiki.mozilla.org/Irc) most hours of most days. Be sure to mention this bug number so we can see the context.
3) Build your change with `mach build` and test your change with `mach test toolkit/components/telemetry/tests/`. Also check your changes for adherence to our style guidelines by using `mach lint`
4) Submit the patch for review. Mark me as a reviewer so I'll get an email to come look at your code.
- Here's the guide: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
5) After a series of reviews and changes to your patch, I'll ask Phabricator to push it to autoland. Your code will soon be shipping to Firefox users worldwide!
6) ...now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.

[1]: https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/toolkit/components/telemetry/core/TelemetryScalar.cpp#2274

Comment 3

[spectre0phantom]

Can I help with this bug ?

Comment 4

[Chris H-C :chutten]

You certainly may! Please let me know if you have any questions. (sorry for the delayed reply. I've been on holidays)

Comment 5

[Chris H-C :chutten]

Unassigning due to inactivity. If you're still working on this bug, spectre0phantom, just let me know and I'll reassign.

Comment 6

[ayushnathjha]

I want to help with this bug.

Comment 7

[Chris H-C :chutten]

Great! I have assigned it to you. Let me know if you have any questions.

Comment 8

[ayushnathjha]

@Chris H-C :chutten
I have followed all the steps required to Build firefox for Windows
hg clone https://hg.mozilla.org/mozilla-central is not working
I request you to guide me how to proceed further

Comment 9

[Chris H-C :chutten]

It might help to relate your error to the #introduction channel on IRC (setup instructions can be found here: https://wiki.mozilla.org/Irc).

I'm afraid without an error message there's rather little that I can probably help with. At a guess, maybe you don't have mercurial (the hg part) installed? (Though I think mozilla-build is supposed to make sure that's working for you).

Or maybe there's some firewall/network configuration error keeping you from accessing https://hg.mozilla.org?

Comment 10

[ayushnathjha]

@Chris H-C:chutten
I have successfully built the Firefox source for Desktop.
I have accessed the TelemetryScalar.cpp file in the mozilla-central directory and made the required changes.
Now how do I test and commit the change? I request you to help me with this.

Comment 11

[Chris H-C :chutten]

To run some sanity tests to make sure things are going well, run mach test toolkit/components/telemetry/tests/

To ensure your code meets the code style guide, run mach lint

To submit your patch for review, please follow the guide here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Comment 12

[ayushnathjha]

@Chris H-C:chutten

Can I get any info of the reviewers of the patch that I am going to create for this Bug?

Comment 13

[Chris H-C :chutten]

Sure! You should mark me as the reviewer so I'll get an email to come look at your code.

Comment 14

[Chris H-C :chutten]

Marking unassigned after two weeks inactivity. (ayushnathjha, if you're still working on this please let us know and we'll reassign it to you, no worries).

Comment 15

[Jason Ashton]

Attachment: Bug 1513546 - Change error message for GetKeyedScalarByEnum lookup in ApplyKeyedScalarActions r=chutten

Comment 16

[Jason Ashton]

Hi Chris, I tried my hand at making this change. This is my first time working on Firefox, I've tried to follow the guides the best I can. The patch is on Phabricator here: https://phabricator.services.mozilla.com/D23818. Let me know if there's anything I can do to land my first contribution!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e282ce97725f
Change error message for GetKeyedScalarByEnum lookup in ApplyKeyedScalarActions r=chutten

Comment 18

[Chris H-C :chutten]

That'll do just fine, we can land it from here.

Thank you for your contribution! Do you have something in mind for what you'd like to try for your next bug?

Comment 19

[Jason Ashton]

Thanks Chris! I'd like to work on C++ bugs in Firefox, I'm trying to continue to improve my C++ and work in a large project. I don't have any particular piece of the project in the mind. Is there anything needed that requires a bit more work than this one (maybe requires adding tests)?

Comment 20

[Chris H-C :chutten]

Looks like we have something in bug 1335009 that might be a little more interesting. To test it we have a google test-based suite in toolkit/components/telemetry/gtest that you could add to (though I'm not sure any of the tests there are similar enough to use as templates)

Comment 21

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/e282ce97725f