Closed Bug 1217458 Opened 5 years ago Closed 5 years ago

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

Categories

(Toolkit :: Telemetry, defect, P3)

defect
Points:
1

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox44 --- affected
firefox45 --- fixed

People

(Reporter: Dexter, Assigned: nwokop, Mentored)

References

(Blocks 1 open bug)

Details

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

Attachments

(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] - 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?
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
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.
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]
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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/565639d2a1b8
Status: ASSIGNED → RESOLVED
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.