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:
1)
(
name,
surname, # pylint: disable=unused-variable
gender
) = user['mbasti']
I dont like this approach
2)
Use defined keyword: 'dummy' is default in pylint, we can set our
own, like ignored, unused
name, dummy, gender = user['mbasti']
3)
use a prefix for unused variables: '_' or 'ignore_'
name, _surname, gender = user['mbasti']
4)
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
Martin^2
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:
https://github.com/freeipa/freeipa/pull/97/commits/06f35e5bdcb9f3ea42145de253674fda06b43d30
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:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
