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

[David Kupka]

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.

-- 
David Kupka