Last Comment Bug 722168 - convert mailnews/extensions/offline-startup/js/offlineStartup.js to Services.jsm
: convert mailnews/extensions/offline-startup/js/offlineStartup.js to Services.jsm
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 13.0
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks: 720356
  Show dependency treegraph
 
Reported: 2012-01-29 09:21 PST by :aceman
Modified: 2012-02-13 06:27 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (6.20 KB, patch)
2012-01-29 09:50 PST, :aceman
standard8: review-
Details | Diff | Review
patch v2 (8.42 KB, patch)
2012-02-08 10:36 PST, :aceman
standard8: review+
Details | Diff | Review

Description :aceman 2012-01-29 09:21:16 PST

    
Comment 1 :aceman 2012-01-29 09:50:57 PST
Created attachment 592516 [details] [diff] [review]
patch
Comment 2 Mark Banner (:standard8) 2012-02-07 15:21:11 PST
Comment on attachment 592516 [details] [diff] [review]
patch

Review of attachment 592516 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/extensions/offline-startup/js/offlineStartup.js
@@ +40,4 @@
>  Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
>  
> +const kDebug              = false;
> +const kBundleURI          = "chrome://messenger/locale/offlineStartup.properties";

We only use this in one place, so there's no point in this being a const.

@@ +67,5 @@
>    onProfileStartup: function()
>    {
>      debug("onProfileStartup");
>  
> +    var ioService = Services.io;

Having had a discussion about this, I think we should drop the intermediate variable/alias - it'll be clearer to everyone and javascript should optimise nicely anyway.

Ditto with prefs and promptService

@@ +115,2 @@
>  
>        var bundle = getBundle(kBundleURI);

We should just kill the getBundle function - we don't do that intense checking elsewhere, and if getting the string bundle is failing, then we've got automation issues somewhere as it should have been picked up.
Comment 3 :aceman 2012-02-08 10:36:46 PST
Created attachment 595451 [details] [diff] [review]
patch v2

Fixes nits.
Comment 4 Mark Banner (:standard8) 2012-02-13 06:27:57 PST
Checked in: http://hg.mozilla.org/comm-central/rev/d4652eaa1a30

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