Closed Bug 1217458 Opened 5 years ago Closed 5 years ago

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


(Toolkit :: Telemetry, defect, P3)




Tracking Status
firefox44 --- affected
firefox45 --- fixed


(Reporter: Dexter, Assigned: nwokop, Mentored)


(Blocks 1 open bug)


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


(1 file, 3 obsolete files)

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] -
No longer depends on: 1217452
No longer depends on: 1217448, 1217457
Blocks: 1201022
Priority: P4 → P3
The comments that need to be changed are:


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?
Attached 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
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.
Attached patch bug1217458_editTEXT2.diff (obsolete) — Splinter Review
Final Revision - Comment ammended.
Attachment #8696558 - Flags: review?(alessio.placitelli)
Comment on attachment 8696558 [details] [diff] [review]

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]

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+
Keywords: checkin-needed
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.