Issue 4096 - remove qunit/* from other builds than devenv

Issue 4096 - remove qunit/* from other builds than devenv

[tlucas, 2017-08-02 13:50:55]

Patch Set 1:

re-indented metadata-testScript to only happen in devenv (excludes qunit/* from
any other build)

[Vasily Kuznetsov, 2017-08-02 17:07:56]

Hi Tristan,

It looks good, although I haven't actually run the code (I assume you did that
and checked the resulting archive). I have one comment about the formatting (see
below).

Cheers,
Vasily

https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py
File packagerChrome.py (right):

https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py#n...
packagerChrome.py:397: params, 'testIndex.html.tmpl', ('general',
'testScripts'))
If you run tox, flake8 fails on this line. This can be fixed by un-indenting it
one step, but it would be preferable (although not yet documented in
https://adblockplus.org/coding-style#python) to do something like this:

            files['qunit/index.html'] = createScriptPage(
                params, 'testIndex.html.tmpl', ('general', 'testScripts')
            )

[Jon Sonesen, 2017-08-02 17:46:51]

On 2017/08/02 17:07:56, Vasily Kuznetsov wrote:
> Hi Tristan,
> 
> It looks good, although I haven't actually run the code (I assume you did that
> and checked the resulting archive). I have one comment about the formatting
(see
> below).
> 
> Cheers,
> Vasily
> 
> https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py
> File packagerChrome.py (right):
> 
>
https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py#n...
> packagerChrome.py:397: params, 'testIndex.html.tmpl', ('general',
> 'testScripts'))
> If you run tox, flake8 fails on this line. This can be fixed by un-indenting
it
> one step, but it would be preferable (although not yet documented in
> https://adblockplus.org/coding-style#python) to do something like this:
> 
>             files['qunit/index.html'] = createScriptPage(
>                 params, 'testIndex.html.tmpl', ('general', 'testScripts')
>             )

Hey Tristan, 

I agree with Vasily, perhaps you can install a flaking plugin into your
preferred text editor?

[tlucas, 2017-08-02 18:05:08]

On 2017/08/02 17:46:51, Jon Sonesen wrote:
> On 2017/08/02 17:07:56, Vasily Kuznetsov wrote:
> > Hi Tristan,
> > 
> > It looks good, although I haven't actually run the code (I assume you did
that
> > and checked the resulting archive). I have one comment about the formatting
> (see
> > below).
> > 
> > Cheers,
> > Vasily
> > 
> > https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py
> > File packagerChrome.py (right):
> > 
> >
>
https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py#n...
> > packagerChrome.py:397: params, 'testIndex.html.tmpl', ('general',
> > 'testScripts'))
> > If you run tox, flake8 fails on this line. This can be fixed by un-indenting
> it
> > one step, but it would be preferable (although not yet documented in
> > https://adblockplus.org/coding-style#python) to do something like this:
> > 
> >             files['qunit/index.html'] = createScriptPage(
> >                 params, 'testIndex.html.tmpl', ('general', 'testScripts')
> >             )
> 
> Hey Tristan, 
> 
> I agree with Vasily, perhaps you can install a flaking plugin into your
> preferred text editor?

As a matter of fact, that was about the second thing i did... unfortunately, i
did not yet get it to work with the configured stuff-to-ignore from the tox.ini
- i assume i have simply overseen that warning in gvim. I think i'll add a
tox-run to my upload.py, for good measure - as soon as i get my hands on my
dev-machine again!

[tlucas, 2017-08-03 10:00:22]

Patch Set 2

Resolved pep8-indentation error

https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py
File packagerChrome.py (right):

https://codereview.adblockplus.org/29503597/diff/29503598/packagerChrome.py#n...
packagerChrome.py:397: params, 'testIndex.html.tmpl', ('general',
'testScripts'))
On 2017/08/02 17:07:56, Vasily Kuznetsov wrote:
> If you run tox, flake8 fails on this line. This can be fixed by un-indenting
it
> one step, but it would be preferable (although not yet documented in
> https://adblockplus.org/coding-style#python) to do something like this:
> 
>             files['qunit/index.html'] = createScriptPage(
>                 params, 'testIndex.html.tmpl', ('general', 'testScripts')
>             )

Done - also got vim-flake8 running with consindering tox.ini config

[Vasily Kuznetsov, 2017-08-03 10:35:48]

LGTM

[kzar, 2017-08-04 12:28:06]

LGTM too, email me over a patch when you get a chance and I'll push it for you.