On 22.09.2016 18:05, Tomas Krizek wrote:
On 09/22/2016 06:00 PM, Martin Basti wrote:

On 22.09.2016 17:59, Tomas Krizek wrote:
On 09/22/2016 04:39 PM, Martin Basti wrote:
Hello all,

In 4.5, I would like to remove all unused variables from code and enable pylint check. Due to big amount of unused variables in the code this will be longterm effort.

Why this?:

* better code readability

* removing dead code

* unused variable may uncover potential bug

It is clear what to do with unused assignments, but I need an agreement what to do with unpacking or iteration with unused variables

For example:

for name, surname, gender in (('Martin', 'Basti', 'M'), ):

name, surname, gender = user['mbasti']

Where 'surname' is unused

Pylint will detect surname as unused variable and we have to agree on a way how to tell pylint that this variable is unused on purpose:




   surname,  # pylint: disable=unused-variable


) = user['mbasti']

I dont like this approach


Use defined keyword: 'dummy' is default in pylint, we can set our own, like ignored, unused

name, dummy, gender = user['mbasti']


use a prefix for unused variables: '_' or 'ignore_'

name, _surname, gender = user['mbasti']


we can combine all :)

For me the best is to have prefix '_' and 'dummy' keyword

As first step I'll enable pylint check and disable it locally per module where unused variables are, to avoid new regressions. Then I will fix it module by module.

I'm open to suggestions


I'd use a double underscore variable:

name, __, gender = user['mbasti']

It is quicker to write than _dummy and it also provides a better readability, because I can immediately identify the symbol as special. Unlike _dummy which I have to read to understand (simply because I'm used to _something to indicate a 'protected' variable).

With double underscores there is a risk, that typo will be just one underscore and _ means ugettext in IPA :)
I think pylint would detect a redefinition in that case, since this check was added in today's PR#97:


So much trust in pylint :), I tested it and it passed with redefinition of '_'. It works only for some cases.

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

Reply via email to