Noissue - Removed background.js

Noissue - Removed background.js

[Sebastian Noack, 2018-04-14 12:30:44]

For reference, the background.js script is historical. It used to include all
code before we started to use modules.

While the "forward" message (the last remaining code in background.js) could be
used more generically in theory, it is only used by the composer, so I moved it
there.

[kzar, 2018-04-16 11:08:22]

> While the "forward" message (the last remaining code in
> background.js) could be used more generically in theory, it is only
> used by the composer, so I moved it there.

Fair enough I guess, but I wonder if we end up needing this message elsewhere
what we'll do then? Where would it live then?

I agree about removing background.js, I'm just not 100% convinced the forward
message handler belongs in lib/filterComposer.js. In fact surely
adblockplusui/messageResponder.js would make more sense?

https://codereview.adblockplus.org/29752571/diff/29752572/lib/filterComposer.js
File lib/filterComposer.js (right):

https://codereview.adblockplus.org/29752571/diff/29752572/lib/filterComposer....
lib/filterComposer.js:315: port.on("forward", (msg, sender) =>
If this is no longer a generic message, it's name should be prefixed "composer."
I guess.

[Sebastian Noack, 2018-04-16 11:17:10]

On 2018/04/16 11:08:22, kzar wrote:
> > While the "forward" message (the last remaining code in
> > background.js) could be used more generically in theory, it is only
> > used by the composer, so I moved it there.
> 
> Fair enough I guess, but I wonder if we end up needing this message elsewhere
> what we'll do then? Where would it live then?
> 
> I agree about removing background.js, I'm just not 100% convinced the forward
> message handler belongs in lib/filterComposer.js. In fact surely
> adblockplusui/messageResponder.js would make more sense?

I'd rather not extend the usage of the "forward" message, it's kinda a hack
after all. And if we should still ever consider using it in another context, we
can revisit where to put it.

As mentioned on IRC, I accidentally already pushed this change, and FWIW I think
the change as-is is an improvement already, and if we agree on any further
refactoring we can address it separately. What do you think?

[kzar, 2018-04-16 11:20:33]

On 2018/04/16 11:17:10, Sebastian Noack wrote:
> As mentioned on IRC, I accidentally already pushed this change, and FWIW I
think
> the change as-is is an improvement already, and if we agree on any further
> refactoring we can address it separately. What do you think?

Well yea, the commit that already got pushed isn't making things any worse at
least. It would be nice if you responded to my feedback though I suppose, though
I don't feel too strongly about it.

[Sebastian Noack, 2018-04-16 11:24:34]

On 2018/04/16 11:20:33, kzar wrote:
> Well yea, the commit that already got pushed isn't making things any worse at
> least. It would be nice if you responded to my feedback though I suppose,
though
> I don't feel too strongly about it.

I responded to your feedback above (explaining why I'm not concerned about the
theoretical use case of that message in other/future scenarios). I just didn't
explicitly reply about renaming the message to "composer.forward". I planned to
submit a separate patch for that.

[kzar, 2018-04-16 11:46:30]

Along with your follow up changes this LGTM