Fix the TODO comments in TelemetrySession.jsm to point to the correct bug

RESOLVED FIXED in Firefox 45

Status

()

defect
P3
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Dexter, Assigned: nwokop, Mentored)

Tracking

(Blocks 1 bug)

Trunk
mozilla45
Points:
1
Dependency tree / graph

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)

Details

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

Attachments

(1 attachment, 3 obsolete attachments)

We need to change the "TODO: Remove this when bug 1124128 lands" comments in TelemetrySession.jsm [1] to "TODO: Remove this when bug 1201837 lands", as bug 1124128  was closed.

[1] - https://dxr.mozilla.org/mozilla-central/rev/daa7d98525e859d32a3b3e97101e129a897192a1/toolkit/components/telemetry/TelemetrySession.jsm#1070
No longer depends on: 1217452
No longer depends on: 1217448, 1217457
Blocks: 1201022
Priority: P4 → P3
The comments that need to be changed are:

- https://dxr.mozilla.org/mozilla-central/rev/7883e81f3c305078353ca27a6b1adb8c769d5904/toolkit/components/telemetry/TelemetrySession.jsm#972
- https://dxr.mozilla.org/mozilla-central/rev/7883e81f3c305078353ca27a6b1adb8c769d5904/toolkit/components/telemetry/TelemetrySession.jsm#976

To check that everything works correctly (even though we're only changing a comment), run the following command:

./mach xpcshell-test toolkit/components/telemetry
Mentor: alessio.placitelli
Points: --- → 1
Whiteboard: [measurement:client] → [measurement:client][lang=js][good first bug]
Hi, I would be submitting a patch for this bug within the next few days. Would it be OK to assign this bug to me after patch submission please?
Posted patch rev1 - Replace line of text (obsolete) — Splinter Review
Replaced line as requested. Please check that everythuing is ok.
Attachment #8695548 - Flags: review?(alessio.placitelli)
My first patch had a change in just #972 as i missed adding the changes in #976. The second patch covers both.
Attachment #8695797 - Flags: review?(alessio.placitelli)
Assignee: nobody → nwokop
Status: NEW → ASSIGNED
Attachment #8695797 - Flags: review?(alessio.placitelli)
Attachment #8695548 - Flags: review?(alessio.placitelli)
(In reply to Patrick from comment #2)
> Hi, I would be submitting a patch for this bug within the next few days.
> Would it be OK to assign this bug to me after patch submission please?

Hi Patrick, welcome and thank you for submitting your first patches!

Do you mind merging them in a single patch file? Note that you can use the title of this bug as a commit message in the bug.

Once done, please attach the new patch and flag me for review!
Hi Alessio. Sure I'll upload another patch soon.
Posted patch bug1217458_editTEXT2.diff (obsolete) — Splinter Review
Final Revision - Comment ammended.
Attachment #8696558 - Flags: review?(alessio.placitelli)
Comment on attachment 8696558 [details] [diff] [review]
bug1217458_editTEXT2.diff

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

We're almost there, thanks for the update. Could you change the commit message to "Bug 1217458: change TelemetrySession comments to point to the correct bug number.". Once done that, we'll land this patch!
Attachment #8696558 - Flags: review?(alessio.placitelli) → feedback+
Attachment #8695548 - Attachment is obsolete: true
Attachment #8695797 - Attachment is obsolete: true
I think that's everything. :)
Attachment #8696661 - Flags: review?(alessio.placitelli)
Attachment #8696558 - Attachment is obsolete: true
Comment on attachment 8696661 [details] [diff] [review]
bug1217458_editTEXT2.diff

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

This looks good now, thanks.

We can push that, as this only changes a comment.
Attachment #8696661 - Flags: review?(alessio.placitelli) → review+
https://hg.mozilla.org/mozilla-central/rev/565639d2a1b8
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.