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

[Martin Basti]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0407.4-make-lint-use-config-file-and-plugin-for-pylint.patch
Type: text/x-patch
Size: 26712 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160209/28f420f3/attachment.bin>