Closed Bug 1301627 Opened 3 years ago Closed 3 years ago
tools/power/* comments don't match the parameter names
58 bytes, text/x-review-board-request
777 bytes, patch
|Details | Diff | Splinter Review|
This is a trivial bug. This has been found by clang-tidy ( http://clang.llvm.org/extra/clang-tidy/checks/misc-argument-comment.html ). The idea is to give an easy bug for someone who wants to understand how to contribute to Firefox. tools/power/rapl.cpp @ Line 443 argument name 'cpu' in comment does not match parameter name 'aCpu' tools/power/rapl.cpp @ Line 443 argument name 'pid' in comment does not match parameter name 'aPid' tools/power/rapl.cpp @ Line 444 argument name 'flags' in comment does not match parameter name 'aFlags' tools/power/rapl.cpp @ Line 444 argument name 'group_fd' in comment does not match parameter name 'aGroupFd'
Summary: tools/power/* Comments doesn't match the parameter names → tools/power/* comments don't match the parameter names
Hi, request to assign this bug to me. I have another similar bug. I can fix this one as well. Thanks
As it is a bug to understand our workflow and it is not about programming, I would prefer to leave this one for someone else. Sorry
Fair enough. Having someone else fix this to understand the workflow makes sense. Thanks.
Hello! I just started working on bugs here, Can someone take me through the process of taking this bug and fixing it ?
This bug is already assigned and being worked on. Thanks.
My apologies. Wrong update. This bug is available. Please go ahead. I would recommend installing the build vm: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Using_the_VM#Working_in_the_VM Please have a look at the developer guide: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide
@umesh are there any ways other than installing the build ?
I am a newbie and so far I have been using the VM. I guess you need an environment to do the build to change code.
Hii, I am new here.I would like to work on this bug.Can anyone tell me how to proceed ?
Hi, I am new here and I request to assign this bug to me.
Is anyone working on this bug ?
I will assign it to the person who provides a patch
@sylvestre how do we go about doing that exactly?
I was trying to submit a patch on mozreview but it is showing me this error abort: cannot submit reviews referencing multiple bugs (limit reviewed changesets with "-c" or "-r" arguments) can you help me with that ?
review flag is not set on your latest patch. if the patch is ready, can you set review flag?
This looks good but I am not the component owner. You will have to find him/her! :)
Assignee: nobody → singlakirti3
Attachment #8793852 - Flags: review?(sledru) → feedback+
I am a new mozilla contributor so can u guide me through further steps.
you can ask a review from someone who has reviewed the file several times, or who reviewed the part you've touched. you can check reviewer in commit log for the file. https://hg.mozilla.org/mozilla-central/filelog/f0f1aaf051d6798e1e73d1feee07ca847333167a/tools/power/rapl.cpp or hg annotate https://hg.mozilla.org/mozilla-central/annotate/f0f1aaf051d6798e1e73d1feee07ca847333167a/tools/power/rapl.cpp in this case, erahm will be suitable reviewer.
Comment on attachment 8793852 [details] [diff] [review] changed the name of variables in the comment Review of attachment 8793852 [details] [diff] [review]: ----------------------------------------------------------------- This is a great start, I'm going to clear the review request for now. Can you update the param names and add a new patch with the changes? ::: tools/power/rapl.cpp @@ +518,5 @@ > attr.config = config; > > // Measure all processes/threads. The specified CPU doesn't matter. > + mFd = perf_event_open(&attr, /* apid = */ -1, /* acpu = */ 0, > + /* agroup_fd = */ -1, /* aflags = */ 0); I think the tool wants you to use the exact param names, you can see those in the |perf_event_open| definition  (aPid, aCpu, etc).  https://dxr.mozilla.org/mozilla-central/rev/f0f1aaf051d6798e1e73d1feee07ca847333167a/tools/power/rapl.cpp#443
Due to system failure I need to rebuild firefox on my machine, so I will upload the new patch as soon as my machine is up and running again.
Comment on attachment 8803715 [details] [diff] [review] Bug 1301627 - Changed the name of variables in the comment on line 521 Review of attachment 8803715 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, thank you for your contribution! I'll go ahead and land it for you.
Attachment #8803715 - Flags: review?(erahm) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/93cdd187a2535349195fd5d4ec71bee88fa04f54 Bug 1301627 - Change the name of commented variables when calling perf_event_open. r=erahm
Kirti, I landed the change for you with an updated commit message. We like to have the name of the reviewer added to the commit message (the "r=erahm" part) and a made the message a bit more generic. I look forward to seeing your next patch!
(In reply to Eric Rahm [:erahm] from comment #29) > Kirti, I landed the change for you with an updated commit message. We like > to have the name of the reviewer added to the commit message (the "r=erahm" > part) and a made the message a bit more generic. I look forward to seeing > your next patch! Thanks a lot for your help and support. I wanted to know if the work on this bug is complete or is there something more to be done.
You need to log in before you can comment on or make changes to this bug.