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

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 1

[velocirabbit]

I've started working on this one. Will update with questions/concerns/patch.

Comment 2

[velocirabbit]

Attachment: Edits _SessionFile.jsm

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.

Comment 3

[velocirabbit]

Attachment: This one should be more correct as a patch.

This one should be more correct as a patch. I went through and edited it.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

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!

Comment 5

[velocirabbit]

Attachment: Sorry about that! Try now.

Comment 6

[velocirabbit]

Did that patch work?

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 8

[velocirabbit]

Attachment: Bug 904480 - Test bug: r=reviewer

checkin-needed

Comment 9

[velocirabbit]

Like that? There wasn't a field specifically for keywords.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

That's not quite right, sorry. Please see bug 901126 comment #20 on how to do this.

Comment 11

[velocirabbit]

Attachment: bug904480.patch

Sorry about that! These are my first bugs/patches. Should be all good now.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/aa83c937e7a9

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aa83c937e7a9