1225103 - Fix the wrong comment for |TelemetryController._getApplicationSection|

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P4)

Description

[Alessio Placitelli [:Dexter]]

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

Comment 1

[Alessio Placitelli [:Dexter]]

Hi Tom! Would you be interested in taking this one?

Comment 2

[Tom Zhang]

Attachment: Patchv1

Sure I'll give it a shot. Here you go.

Comment 3

[Tom Zhang]

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?

Comment 4

[Alessio Placitelli [:Dexter]]

Comment on attachment 8688249 [details] [diff] [review]
Patchv1

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

This looks good, thanks!

Comment 5

[Alessio Placitelli [:Dexter]]

(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!

Comment 6

[Alessio Placitelli [:Dexter]]

Tom, did you make sure the tests run locally with no problem? If not, please try "./mach xpcshell-test toolkit/components/telemetry"

Comment 7

[Tom Zhang]

I've just ran the tests. I had to run with the sequential flag, but all tests appear to have passed.

Comment 8

[Alessio Placitelli [:Dexter]]

(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.

Comment 9

[Alessio Placitelli [:Dexter]]

https://hg.mozilla.org/integration/fx-team/rev/13f5e7a3abece2427b8af7b0905c7fedfc95531a
Bug 1225103 - Clarity change to debug message to reflect actual function called. r=dexter

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/13f5e7a3abec