Noissue - Fix a break when loading adblockpluscore into Chrome extension

Noissue - Fix a break when loading adblockpluscore into Chrome extension

[hub, 2017-03-29 21:12:16]

I was trying to load a more recent version of adblockpluscore into the chrome
extension.

This is caused by 68a4dc4e79e8

[sergei, 2017-03-29 21:52:14]

Adding myself to CC because of curiosity because this issue is also valid for
libadblockplus and the proposed change should also fix it for libadblockplus.

[kzar, 2017-03-30 05:13:32]

NOT LGTM

Please see the ESLint adblockpluschrome review[1]. If it helps here's[2] my
branch in GitHub, perhaps base your branch on that? Heads up though I do
force-push to that branch quite a bit, it's still in flux, depending on your
tooling that might be a pain.

Some other feedback:

1. I'm not sure this qualifies as a no issue change, but even if it does it at
least needed more context in the review description. It took me a minute to
figure out what you were trying to do, why it didn't work and therefore the
reason for your changes. It didn't help that the title was vague too.
2. Please don't just assume I've broken something and call me out in
#adblockplus without at least talking to me / someone about it. Maybe I'm overly
sensitive but I found kind of rude. I'm sure you didn't mean for that. How about
"kzar: It seems that when I update the adblockpluscore dependency of
adblockpluschrome to include your recent ESLint changes things break, any
ideas?"?
3. It's adblockpluscore not abpcore, we generally avoid the ABP abbreviation
where possible and plus in this case it's a repository name too.
4. When rebasing changes under review please upload a separate patch set, that
makes it clear to the reviewer which changes are down to the rebase. If you
forget then at least add (an) inline comment(s) explaining the following changes
are due to rebase.

One thing I personally could have done better is to have added an integration
note about the Cu.import change to the issue[3], perhaps that would help helped
avoid confusion. I'll do that now.

[1] https://codereview.adblockplus.org/29374674/
[2] https://github.com/kzar/adblockpluschrome/tree/4864-eslint
[3] https://issues.adblockplus.org/ticket/4878

[kzar, 2017-03-30 05:16:07]

Oh, one other thing. It's not a hard rule but it's quite convenient for the
reviewer if you start the message with the patch set name like "Patch Set 2 :
Added eslint hints" when publishing a patch set. For larger reviews it makes it
clearer when changes happened relative to the discussion.

[kzar, 2017-03-30 05:30:07]

https://codereview.adblockplus.org/29397663/diff/29397668/lib/filterListener.js
File lib/filterListener.js (right):

https://codereview.adblockplus.org/29397663/diff/29397668/lib/filterListener....
lib/filterListener.js:26: Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Nit: It seems you changed the order of these imports by mistake?