Closed Bug 662253 Opened 11 years ago Closed 11 years ago

Add FileUtils.openFileOutputStream to open non-safe file output streams

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: philikon, Assigned: philikon)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 1 obsolete file)

FileUtils.openSafeFileOutputStream is pretty nice and all, but you don't always need a safe output stream (e.g. for logging)
Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #537535 - Flags: review?
Attachment #537535 - Flags: review? → review?(gavin.sharp)
Attached patch v1.1Splinter Review
Bah, forgot to refresh the patch with some changes in the working copy.
Attachment #537535 - Attachment is obsolete: true
Attachment #537535 - Flags: review?(gavin.sharp)
Attachment #537536 - Flags: review?(gavin.sharp)
Comment on attachment 537536 [details] [diff] [review]
v1.1

nit: s/FileUtils/this/? These methods already depend on |this| being sane for the constants.
Attachment #537536 - Flags: review?(gavin.sharp) → review+
Might be worth making a closeFileOutputStream (that does the same as closeSafeFileOutputStream) for consistency.
(In reply to comment #4)
> Might be worth making a closeFileOutputStream (that does the same as
> closeSafeFileOutputStream) for consistency.

Hmm. What besides stream.close() would it do?
Nothing (just like closeSafeFileOutputStream does for normal OutputStreams)
(In reply to comment #6)
> Nothing (just like closeSafeFileOutputStream does for normal OutputStreams)

Hmm, maybe we should just rename closeSafeFileOutputStream since it handles both cases? Leaving this for a follow-up or a separate patch (small bugs/patches FTW :))

Pushed v1.1 to s-c with Gavin's review comment (comment 3) addressed:
http://hg.mozilla.org/services/services-central/rev/b0e0455d5102
Whiteboard: [fixed in services]
Blocks: 610832
http://hg.mozilla.org/mozilla-central/rev/b0e0455d5102
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: dev-doc-needed
Resolution: --- → FIXED
Whiteboard: [fixed in services]
Target Milestone: --- → mozilla7
Documentation updated by Trevor.
Verified the changes from v1.1 were pushed to mozilla repository; files 
toolkit/mozapps/shared/FileUtils.jsm 
and 
toolkit/mozapps/shared/test/unit/test_FileUtils.js 
are updated.

Is this enough to set the resolution to verified-fixed? Or are there any other tests that need to be run to verify this?

Thank you!
You need to log in before you can comment on or make changes to this bug.