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

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Philipp von Weitershausen [:philikon]]

FileUtils.openSafeFileOutputStream is pretty nice and all, but you don't always need a safe output stream (e.g. for logging)

Comment 1

[Philipp von Weitershausen [:philikon]]

Attachment: v1

Comment 2

[Philipp von Weitershausen [:philikon]]

Attachment: v1.1

Bah, forgot to refresh the patch with some changes in the working copy.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 537536 [details] [diff] [review]
v1.1

nit: s/FileUtils/this/? These methods already depend on |this| being sane for the constants.

Comment 4

[Dave Townsend [:mossop]]

Might be worth making a closeFileOutputStream (that does the same as closeSafeFileOutputStream) for consistency.

Comment 5

[Philipp von Weitershausen [:philikon]]

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

Comment 6

[Dave Townsend [:mossop]]

Nothing (just like closeSafeFileOutputStream does for normal OutputStreams)

Comment 7

[Philipp von Weitershausen [:philikon]]

(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

Comment 8

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/b0e0455d5102

Comment 9

[Eric Shepherd [:sheppy]]

Documentation updated by Trevor.

Comment 10

[Mihaela Velimiroviciu (:mihaelav)]

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!