diff mbox

[2/4] tox: Remove lint target

Message ID 87twbs5j9l.fsf@possimpible.ozlabs.ibm.com
State Rejected
Headers show

Commit Message

Daniel Axtens Oct. 31, 2016, 8:57 p.m. UTC 
> pylint doesn't work very well with Django, and this is broken.

Interesting. I've been carrying the following local patch:


That gets things going, but I haven't followed-up on the list of things
it throws up, so if you think the list is useless I'm happy to go with
your patch.

Regards,
Daniel

>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Cc: Daniel Axtens <dja@axtens.net>
> ---
>  tox.ini | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/tox.ini b/tox.ini
> index 0836cd5..c700b54 100644
> --- a/tox.ini
> +++ b/tox.ini
> @@ -32,16 +32,6 @@ commands = flake8 {posargs} patchwork patchwork/bin/pwclient
>  ignore = E129, F405
>  exclude = ./patchwork/migrations
>  
> -[testenv:lint]
> -basepython = python2.7
> -deps =
> -    pylint
> -    -r{toxinidir}/requirements-prod.txt
> -commands = pylint patchwork --rcfile=pylint.rc
> -
> -[testenv:venv]
> -commands = {posargs}
> -
>  [testenv:coverage]
>  basepython = python2.7
>  deps =
> @@ -54,3 +44,6 @@ commands =
>      coverage run --omit=*tox*,patchwork/tests/*.py,manage.py,patchwork/migrations/*.py \
>           --branch {toxinidir}/manage.py test --noinput patchwork
>      coverage report -m
> +
> +[testenv:venv]
> +commands = {posargs}
> -- 
> 2.7.4

Comments

Stephen Finucane Nov. 4, 2016, 2:37 p.m. UTC | #1 
On 2016-10-31 20:57, Daniel Axtens wrote:
>> pylint doesn't work very well with Django, and this is broken.
> 
> Interesting. I've been carrying the following local patch:
> 
> diff --git a/tox.ini b/tox.ini
> index 0836cd52ae1d..940daf01e29e 100644
> --- a/tox.ini
> +++ b/tox.ini
> @@ -36,7 +36,7 @@ exclude = ./patchwork/migrations
>  basepython = python2.7
>  deps =
>      pylint
> -    -r{toxinidir}/requirements-prod.txt
> +    -r{toxinidir}/requirements-test.txt
>  commands = pylint patchwork --rcfile=pylint.rc
> 
>  [testenv:venv]
> 
> That gets things going, but I haven't followed-up on the list of things
> it throws up, so if you think the list is useless I'm happy to go with
> your patch.

Yeah, it gets things working but there are a lot of false positives. If 
this is something someone's using then I guess we could keep it (after 
applying your patch, that is)? In any case, I've the other patches in 
this series applied.

Stephen
Daniel Axtens Nov. 6, 2016, 11:28 p.m. UTC | #2 
Stephen Finucane <stephen@that.guru> writes:

> On 2016-10-31 20:57, Daniel Axtens wrote:
>>> pylint doesn't work very well with Django, and this is broken.
>> 
>> Interesting. I've been carrying the following local patch:
>> 
>> diff --git a/tox.ini b/tox.ini
>> index 0836cd52ae1d..940daf01e29e 100644
>> --- a/tox.ini
>> +++ b/tox.ini
>> @@ -36,7 +36,7 @@ exclude = ./patchwork/migrations
>>  basepython = python2.7
>>  deps =
>>      pylint
>> -    -r{toxinidir}/requirements-prod.txt
>> +    -r{toxinidir}/requirements-test.txt
>>  commands = pylint patchwork --rcfile=pylint.rc
>> 
>>  [testenv:venv]
>> 
>> That gets things going, but I haven't followed-up on the list of things
>> it throws up, so if you think the list is useless I'm happy to go with
>> your patch.
>
> Yeah, it gets things working but there are a lot of false positives. If 
> this is something someone's using then I guess we could keep it (after 
> applying your patch, that is)? In any case, I've the other patches in 
> this series applied.
>

I haven't gone though the results so I wouldn't really know. Your call -
we already have flake8 so I don't really mind if we drop down to a
single linter.

Regards,
Daniel

> Stephen
diff mbox

Patch

diff --git a/tox.ini b/tox.ini
index 0836cd52ae1d..940daf01e29e 100644
--- a/tox.ini
+++ b/tox.ini
@@ -36,7 +36,7 @@  exclude = ./patchwork/migrations
 basepython = python2.7
 deps =
     pylint
-    -r{toxinidir}/requirements-prod.txt
+    -r{toxinidir}/requirements-test.txt
 commands = pylint patchwork --rcfile=pylint.rc
 
 [testenv:venv]