Noissue - fix according to coding style

add 'use strict'; and we don't use Cu.import to import into global scope.

#project abpcrawler

[sergei, 2016-09-29 09:56:39]

Should it include something else, like change let to const?

[Wladimir Palant, 2016-09-29 10:35:45]

On 2016/09/29 09:56:39, sergei wrote:
> Should it include something else, like change let to const?

No, please don't change variables to constants arbitrarily.

The title of your commit should say "Noissue" - without a space. Generally, I
don't think that this title properly reflects your changes however. You made
several unrelated modifications which would be way better off in separate
commits:

* Add "use strict" to the JavaScript files.
* Don't import JavaScript modules into the global scope (includes removing
unused module).
* Remove usage of the deprecated Promises.jsm module.

Could you separate these, ideally with a separate review for each one?

[Wladimir Palant, 2016-09-29 10:36:10]

https://codereview.adblockplus.org/29355247/diff/29355248/lib/commandLine.js
File lib/commandLine.js (right):

https://codereview.adblockplus.org/29355247/diff/29355248/lib/commandLine.js#...
lib/commandLine.js:7: 'use strict';
Double quotation marks please (same for the other files).

https://codereview.adblockplus.org/29355247/diff/29355248/lib/crawler.js
File lib/crawler.js (right):

https://codereview.adblockplus.org/29355247/diff/29355248/lib/crawler.js#newc...
lib/crawler.js:14: const {Task} = Cu.import("resource://gre/modules/Task.jsm");
Please add the second parameter, otherwise it still add variables to global
scope.

https://codereview.adblockplus.org/29355247/diff/29355248/lib/main.js
File lib/main.js (right):

https://codereview.adblockplus.org/29355247/diff/29355248/lib/main.js#newcode13
lib/main.js:13: const {XPCOMUtils} =
Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
Why did you remove Services.jsm? It is being used here.

Note that the code still works because the global Services variable from
bootstrap.js is leaking through - that's a bug.

https://codereview.adblockplus.org/29355247/diff/29355248/lib/main.js#newcode40
lib/main.js:40: return windowsRestored;
This is not how the code should be structured - you should rather move
everything into the callback:

  return new Promise((resolve, reject) =>
  {
    let observer = ...;
    addObserver(...);
    onShutdown.add(...);
  });

Also, please don't leave out the reject parameter even though you aren't using
it.

[sergei, 2016-09-29 12:46:17]

I am closing this issue in favor of
-  29355276: Noissue - Remove usage of the deprecated Promises.jsm module.
- 29355271: Noissue - Don't import JavaScript modules into the global scope
(includes removing unused module).
- 29355266: Noissue - Add "use strict" to the JavaScript files.

https://codereview.adblockplus.org/29355247/diff/29355248/lib/commandLine.js
File lib/commandLine.js (right):

https://codereview.adblockplus.org/29355247/diff/29355248/lib/commandLine.js#...
lib/commandLine.js:7: 'use strict';
On 2016/09/29 10:36:10, Wladimir Palant wrote:
> Double quotation marks please (same for the other files).

Done in https://codereview.adblockplus.org/29355266/.

https://codereview.adblockplus.org/29355247/diff/29355248/lib/crawler.js
File lib/crawler.js (right):

https://codereview.adblockplus.org/29355247/diff/29355248/lib/crawler.js#newc...
lib/crawler.js:14: const {Task} = Cu.import("resource://gre/modules/Task.jsm");
On 2016/09/29 10:36:10, Wladimir Palant wrote:
> Please add the second parameter, otherwise it still add variables to global
> scope.

Done in https://codereview.adblockplus.org/29355271/

https://codereview.adblockplus.org/29355247/diff/29355248/lib/main.js
File lib/main.js (right):

https://codereview.adblockplus.org/29355247/diff/29355248/lib/main.js#newcode40
lib/main.js:40: return windowsRestored;
On 2016/09/29 10:36:10, Wladimir Palant wrote:
> This is not how the code should be structured - you should rather move
> everything into the callback:
> 
>   return new Promise((resolve, reject) =>
>   {
>     let observer = ...;
>     addObserver(...);
>     onShutdown.add(...);
>   });
> 
> Also, please don't leave out the reject parameter even though you aren't using
> it.

Done in https://codereview.adblockplus.org/29355271/