Closed Bug 904480 Opened 11 years ago Closed 11 years ago

Return values of _SessionFile.wipe() and .writeLoadStateOnceAfterStartup() are unused

Categories

(Firefox :: Session Restore, defect)

Product:
Firefox
Component:
Session Restore
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 26

People

(Reporter: ttaubert, Assigned: iforgot120)

Details

(Whiteboard: [good first bug][mentor=ttaubert][lang=js])

Attachments

(1 file, 4 obsolete files)

bug904480.patch
2.57 KB, patch
Details | Diff | Splinter Review
We don't actually use the return values of these two functions, so let's clean this up and turn the 'return' statements into function calls.
No longer depends on: 904477
I've started working on this one. Will update with questions/concerns/patch.
Attached patch Edits _SessionFile.jsm (obsolete) — — Splinter Review
This is my first time making a patch. I had to manually edit this (and another) one because I didn't realize how Mercurial worked so I had the two bug fixes combined into one patch. Hopefully this works, otherwise I can try to edit the patch again or just upload that combined patch.
This one should be more correct as a patch. I went through and edited it.
Comment on attachment 791129 [details] [diff] [review] This one should be more correct as a patch. Review of attachment 791129 [details] [diff] [review]: ----------------------------------------------------------------- Thank you Andrew, that patch looks great. The only problem is that I unfortunately can't apply your patch on my Unix machine - that is caused by DOS line endings. Can you please set your editor to use Unix-style line endings, re-save your files and generate a new patch? Thanks!
Attachment #791129 - Flags: feedback+
Assignee: nobody → iforgot120
Status: NEW → ASSIGNED
Attached patch Sorry about that! Try now. (obsolete) — — Splinter Review
Did that patch work?
Comment on attachment 791531 [details] [diff] [review] Sorry about that! Try now. Review of attachment 791531 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Andrew, this looks great! Can you please prepare the patch for checkin-needed? That means giving it a description like "Bug 12345 - Test bug; r=reviewer", uploading it again and adding the 'checkin-needed' keyword to the bug.
Attachment #791531 - Flags: review+
checkin-needed
Attachment #791118 - Attachment is obsolete: true
Attachment #791129 - Attachment is obsolete: true
Attachment #791531 - Attachment is obsolete: true
Like that? There wasn't a field specifically for keywords.
That's not quite right, sorry. Please see bug 901126 comment #20 on how to do this.
Attached patch bug904480.patch — — Splinter Review
Sorry about that! These are my first bugs/patches. Should be all good now.
Attachment #792770 - Attachment is obsolete: true
Keywords: checkin-needed
(In reply to velocirabbit from comment #11) > Sorry about that! These are my first bugs/patches. Should be all good now. No worries, we all had to get used to that process :) Thanks a lot for your patches, both of them should be landed soon!
https://hg.mozilla.org/integration/fx-team/rev/aa83c937e7a9
Keywords: checkin-needed
Whiteboard: [good first bug][mentor=ttaubert][lang=js] → [good first bug][mentor=ttaubert][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/aa83c937e7a9
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=ttaubert][lang=js][fixed-in-fx-team] → [good first bug][mentor=ttaubert][lang=js]
Target Milestone: --- → Firefox 26
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: