[Freeipa-devel] [PATCH 0407] make lint: migration to config file and pylint plugin due pylint 1.5.2

[David Kupka]

On 09/02/16 11:28, Martin Basti wrote:
>
>
> On 03.02.2016 10:19, David Kupka wrote:
>> On 01/02/16 14:18, Martin Basti wrote:
>>>
>>>
>>> On 26.01.2016 14:16, Martin Basti wrote:
>>>>
>>>>
>>>> On 20.01.2016 14:38, Jan Cholasta wrote:
>>>>> Hi,
>>>>>
>>>>> On 19.1.2016 13:43, Martin Basti wrote:
>>>>>> New pylint version will broke our custom make-lint script again,
>>>>>> attached patch migrates make-lint to:
>>>>>> * config file
>>>>>> * pylint plugin
>>>>>> which are supported by pylint and should not have regular
>>>>>> compatibility
>>>>>> issues
>>>>>>
>>>>>> to test new approach run ./make-lint2
>>>>>>
>>>>>> Advantages:
>>>>>> * compatibility with pylint
>>>>>> * works on both pylint-1.4.3-3.fc23.noarch and
>>>>>> pylint-1.5.2-1.fc24.noarch
>>>>>> * pylint plugin works in different way than the previous custom
>>>>>> checker.
>>>>>> Missing ("dynamic") attributes are added to abstract syntax tree
>>>>>> instead
>>>>>> of ignoring them and all their sub-members. This makes check better,
>>>>>> pylint can detect more typos in tests configurations, api, env, etc..
>>>>>>
>>>>>> Disadvantages:
>>>>>> * any new attribute in api, test config, etc.. must be added to
>>>>>> definition of missing members (pylint plugin) - this should not
>>>>>> happen
>>>>>> too often
>>>>>
>>>>> 1) Please "mv pylint_plugins/fix_ipa_members.py pylint_plugins.py"
>>>>> and "rm -rf pylint_plugins/", no need for this redundant directory
>>>>> structure.
>>>>>
>>>>> 2) Rename pylintrc to freeipa.pylintrc so you have to always specify
>>>>> it explicitly with --rcfile.
>>>>>
>>>>> 3) Use the load-plugins directive in freeipa.pylintrc to load the
>>>>> plugins rather than --load-plugins.
>>>>>
>>>>> 4) Instead of running pylint twice, run it only once with both normal
>>>>> and Python 3 checks enabled:
>>>>>
>>>>>     [MESSAGE CONTROL]
>>>>>     enable=all,python3
>>>>>     disable=...,no-absolute-import
>>>>>
>>>>>>
>>>>>>
>>>>>> Q&TODO:
>>>>>> * make-lint: should it be just bash script or rather python script?
>>>>>
>>>>> IMO neither, it should be a make target (make lint).
>>>>>
>>>>>> * add dynamic detection of python files to be checked
>>>>>
>>>>> You can use "find . -type f -executable ! -path \*/.\* ! -name
>>>>> \*.py\* -exec grep -lsm1 '^#!.*\bpython' \{\} \;".
>>>>>
>>>>>> * should I keep the current options from original make-lint?
>>>>>
>>>>> No, but allow pylint options to be overridable (make lint
>>>>> PYLINTFLAGS="--disable=python3")
>>>>>
>>>>>> * several false positive errors I haven't been able to fix in plugin
>>>>>> yet, in worst case they can be locally disabled:
>>>>>
>>>>> Disable them locally.
>>>>>
>>>>> Honza
>>>>>
>>>> Updated patch attached.
>>>>
>>>> Please note that make-lint script has been removed, to execute lint
>>>> check use 'make lint'
>>>>
>>>>
>>>
>>> Updated patch attached:
>>> * fixes recently added error
>>> * fixes PEP8
>>> * cleanup of pylint config file
>>>
>>> Patch is only for master branch, for 4.3 and 4.2 I will send different
>>> patches when this will be acked
>>>
>>>
>>
>> Could you please add an extensive comment to the 4-lines-long find
>> command? I can after a while (and with the help of man page) decode
>> what it does so it would definitely help to have it described.
>> Otherwise it looks good to me.
>>
> Updated patch attached, only for master, patches for 4.2, 4.3 will
> follow if this one will be ACKed

The comment is probably sufficient, thanks.
Also works for me on current master, ACK.

-- 
David Kupka