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

[Martin Basti]

On 11.02.2016 11:08, Martin Basti wrote:
>
>
> 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
>
Patches for 4.2, 4.3 attached


-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-4-2-mbasti-0407-make-lint-use-config-file-and-plugin-for-pylint.patch
Type: text/x-patch
Size: 25166 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160211/3f717dba/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-4-3-mbasti-0407-make-lint-use-config-file-and-plugin-for-pylint.patch
Type: text/x-patch
Size: 24019 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160211/3f717dba/attachment-0001.bin>