Closed
Bug 1442061
Opened 6 years ago
Closed 6 years ago
Remove content process in memory_total histogram's "record_in_processes"
Categories
(Toolkit :: Telemetry, enhancement, P4)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: bugzilla, Assigned: kylemsguy, Mentored)
References
(Blocks 1 open bug)
Details
(Whiteboard: [good first bug][lang=js])
Attachments
(1 file)
808 bytes,
patch
|
chutten
:
review+
|
Details | Diff | Splinter Review |
Seems like another likely candidate for a histogram that's only recorded in the parent process: https://mzl.la/2HPnS4t
Comment 1•6 years ago
|
||
Indeed it is, good catch. Let's set this up as a mentored bug. 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 editing toolkit/components/telemetry/Histograms.json to remove the "content" notation from the MEMORY_TOTAL probe's record_in_processes field. - 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. 3) Build your change with `mach build` and test your change with `mach test toolkit/components/telemetry/tests/`. You should also run your newly-built Firefox with `mach run` and check the page `about:telemetry` (type it into your address bar) to make sure MEMORY_TOTAL is still recorded. 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 mark it for checkin or 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.
Mentor: chutten
Whiteboard: [good first bug][lang=js]
Updated•6 years ago
|
Priority: -- → P4
Comment 2•6 years ago
|
||
I can take this bug too if possible
Comment 3•6 years ago
|
||
(In reply to Videet Singhai from comment #2) > I can take this bug too if possible We may have a "good second bug" for you after bug 1442402 instead of having you work on a second "good first bug." So let's take it one bug at a time :)
Comment 5•6 years ago
|
||
Okay, I have assigned this bug to you. Please let me know if you have any questions.
Assignee: nobody → kylemsguy
Status: NEW → ASSIGNED
Hello, I've made the change, run the appropriate tests, and submitted the commit: https://reviewboard-hg.mozilla.org/gecko/rev/eecbf329b278
Comment 7•6 years ago
|
||
I am not familiar with reviewboard-hg.mozilla.org. Could you resubmit this either as an attached patch (Splinter review) or using MozReview? The "How to Submit a Patch" guide I linked to in the instructions recommends mozreview: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
Comment 8•6 years ago
|
||
:keylemsguy, are you still working on this? Do you need any help?
Flags: needinfo?(kylemsguy)
Hey sorry, I haven't had the chance to work on this, but I'll give it another shot right now. When I submitted the previous commit, I used `hg push review`, as was suggested in the MozReview documentation. I'm not sure why it's not appearing in MozReview. As for submitting a patch on Bugzilla, I've only just found the link to attach a patch. I'll resubmit my patch via that method.
Flags: needinfo?(kylemsguy)
Comment 10•6 years ago
|
||
Did it have a properly-formatted commit message? Maybe it was missing a bug number and reviewer notation so it wouldn't be able to link it back here or email me... The top line of a commit message should have the number, a short summary, and a r?. Something like: bug 1442061 - Remove "content" from "record_in_processes" of MEMORY_TOTAL r?chutten
Assignee | ||
Comment 11•6 years ago
|
||
Attachment #8962501 -
Flags: review?(chutten)
Assignee | ||
Comment 12•6 years ago
|
||
This was the commit message from my previous attempt: Bug 1442061 - Remove content process in memory_total histogram's "record_in_processes" r?chutten
Comment 13•6 years ago
|
||
Comment on attachment 8962501 [details] [diff] [review] 0001-Bug-1442061-Remove-content-process-in-memory_total-h.patch Review of attachment 8962501 [details] [diff] [review]: ----------------------------------------------------------------- All else fails, splinter review. Thank you for your contribution!
Attachment #8962501 -
Flags: review?(chutten) → review+
Comment 14•6 years ago
|
||
I'll mark this for checkin. Do you need some help finding something to work on next? We have a list of mentored Telemetry bugs over here, if you'd like to take a look: https://georgf.github.io/bugzhub/#mentored
Keywords: checkin-needed
Comment 15•6 years ago
|
||
Pushed by apavel@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/38646fa333aa Remove content process in memory_total histogram's "record_in_processes" r=chutten
Keywords: checkin-needed
Comment 16•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/38646fa333aa
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•