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

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to