Closed Bug 1225103 Opened 4 years ago Closed 4 years ago

Fix the wrong comment for |TelemetryController._getApplicationSection|

Categories

(Toolkit :: Telemetry, defect, P4)

defect
Points:
1

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: Dexter, Assigned: tomzhang94, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [measurement:client][lang=js][good first bug])

Attachments

(1 file)

The comments in [0] and [1] are not reporting the correct function name: they say "assemblePing", while we are in "_getApplicationSection".

We should change the comments to match the correct function name.

[0] - https://dxr.mozilla.org/mozilla-central/rev/bc74dbdea094059d5f1d353a2585b4f6352b6ec4/toolkit/components/telemetry/TelemetryController.jsm#411
[1] - https://dxr.mozilla.org/mozilla-central/rev/bc74dbdea094059d5f1d353a2585b4f6352b6ec4/toolkit/components/telemetry/TelemetryController.jsm#418
Blocks: 1201022
Points: --- → 1
Priority: -- → P4
Whiteboard: [measurement:client][lang=js][good first bug]
Hi Tom! Would you be interested in taking this one?
Flags: needinfo?(tomzhang94)
Attached patch Patchv1Splinter Review
Sure I'll give it a shot. Here you go.
Flags: needinfo?(tomzhang94)
Attachment #8688249 - Flags: review?(alessio.placitelli)
On a side note, I keep on running into the following error while creating a patch:
abort: working directory revision is not qtip

So far I've been forcing mq to think there are no patches applied using > .hg/patches/status. What is the proper way to deal with this error? And the best way to avoid this error in the future?
Assignee: nobody → tomzhang94
Status: NEW → ASSIGNED
Comment on attachment 8688249 [details] [diff] [review]
Patchv1

Review of attachment 8688249 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, thanks!
Attachment #8688249 - Flags: review?(alessio.placitelli) → review+
(In reply to Tom Zhang from comment #3)
> On a side note, I keep on running into the following error while creating a
> patch:
> abort: working directory revision is not qtip
> 
> So far I've been forcing mq to think there are no patches applied using >
> .hg/patches/status. What is the proper way to deal with this error? And the
> best way to avoid this error in the future?

You should pop off all your patches before updating or doing other operations on your repo (hg qpop -a). Then, once done, you can re-apply the patches (hg qpush -a).

To fix the current status you should do |hg update tip| or |hg update --check| (and that *should* fix the issue).

Always popping and then re-applying your patches should prevent this error from appearing again!
Tom, did you make sure the tests run locally with no problem? If not, please try "./mach xpcshell-test toolkit/components/telemetry"
I've just ran the tests. I had to run with the sequential flag, but all tests appear to have passed.
(In reply to Tom Zhang from comment #7)
> I've just ran the tests. I had to run with the sequential flag, but all
> tests appear to have passed.

This is probably not caused by this patch, since it's only changing two strings.
It shouldn't happen, but can happen: don't run the test sequentially and simply try again and it should be ok.
https://hg.mozilla.org/integration/fx-team/rev/13f5e7a3abece2427b8af7b0905c7fedfc95531a
Bug 1225103 - Clarity change to debug message to reflect actual function called. r=dexter
https://hg.mozilla.org/mozilla-central/rev/13f5e7a3abec
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.