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

[Martin Basti]

On 09.02.2016 15:26, David Kupka wrote:
> 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.
>
Pushed to master: 2ce8921fe69ed58871f8e33e3899ad80dcc28c0e

Patches for 4.2 and 4.3 will follow