Re: [PATCH v7 17/31] python: add pylint to pipenv

[John Snow]

On 5/26/21 5:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> 26.05.2021 03:24, John Snow wrote:
> > We are specifying >= pylint 2.8.x for several reasons:
> >
> > 1. For setup.cfg support, added in pylint 2.5.x
> > 2. To specify a version that has incompatibly dropped
> >     bad-whitespace checks (2.6.x)
> > 3. 2.7.x fixes "unsubscriptable" warnings in Python 3.9
> > 4. 2.8.x adds a new, incompatible 'consider-using-with'
> >     warning that must be disabled in some cases.
> >     These pragmas cause warnings themselves in 2.7.x.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
> Not sure how to review this one, numbers looks like numbers, hashes 
> looks like hashes, so it's OK :)

Thanks for going down the list of unreviewed patches :) the change in v7 
from v6 here is requiring pylint > 2.8.0 for the venv, I used to ask 
only for 2.7.0.

I have no idea how to review these lockfiles either. I don't scrutinize 
them too closely -- just as long as they appear to work in the CI 
environment, I am happy.

> I've tried to regenerate Pipfile.lock from venv, and I see that new 
> generated Pipfile.lock has same hashes..
>
> Still, new generated Pipfile.lock has some additional entries: appdirs, 
> distlib, filelock, importlib-resources, packaging, pluggy, py, 
> pyparsing, six, tox, virtualenv.. Not sure is it OK.

If you did that at the *end* of the series, it's because I added tox as 
a development requirement, but didn't update the Pipenv -- because we 
don't actually use tox *in* the venv.

However, I wanted 'pip install .[devel]' to work, so it needs to pull in 
tox there.

Eventually, I'll update the Pipenv for some other reason and it'll 
probably pull in Tox at that point -- maybe a little wasteful, but not 
harmful.

> Another differencies are:
>
> for importlib-metadata:
>
>   "markers": "python_version < '3.8'"   ->   "markers": "python_version 
> < '3.8' and python_version < '3.8'"
>
> (looks like a bug in pipenv, isn't it)

Or at least a version difference. TBQH I do not understand what these 
markers mean, but they DO seem volatile. I decided not to worry about 
them, as long as Pipenv seems to do the right thing ...

> for zipp:
>   "markers": "python_version >= '3.6'"  ->   "markers": "python_version 
> < '3.10'"
>
>
> The thing I hope is: we will not have commits like this one often..

Only when make-check-pipenv starts to fail for some reason and I need to 
update the dependencies. Hopefully not extremely often ... if it becomes 
too much of a pain I will just get rid of it.

I'm not 100% confident this will help more than it hurts yet, but I'll 
find out over the next release or two.

> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Thanks :)