[Buildbot-devel] Running Pylint on Buildbot's source code

[Elmir Jagudin]

On 07/05/2013 04:31 AM, Dustin J. Mitchell wrote:
> :C0112 (empty-docstring): *Empty docstring*
>    Used when a module, function, class or method has an empty docstring (it would
>    be too easy ;). This message belongs to the basic checker.
Is there a point with empty docstrings?

Otherwise I think we should get rid of them, so it will be clear that it 
is not documented.
> Unsure
> ------
>
> :C0102 (blacklisted-name): *Black listed name "%s"*
>    Used when the name is listed in the black list (unauthorized names).
> This message belongs to the basic checker.
This is a configurable list of black listed names. The default is 
'foo,bar,baz,toto,tutu,tata'.

I think this should be fixed, by using more descriptive names.

> :E0102 (function-redefined): *%s already defined line %s*
>    Used when a function / class / method is redefined. This message
> belongs to the basic checker.
> *** pyflakes catches these, so I suspect anything pylint has found is
> a false alarm?

Well, technically speaking these are not false alarms, pylint finds 
these two redefinitions:

[1]: 
https://github.com/buildbot/buildbot/blob/9cd7f9a097f16bc8e0e1f8dbcf52b1946b5c1756/master/buildbot/status/web/change_hook.py#L81
[2]: 
https://github.com/buildbot/buildbot/blob/9cd7f9a097f16bc8e0e1f8dbcf52b1946b5c1756/master/buildbot/db/pool.py#L98

Both of these are actual redefinitions of local variables.

IMHO [1] should be fixed by renaming exception variable on line 63.

Not sure about [2], the code can be changed to:

         log_msg = log.msg
         if verbose:
             def _log_msg(m):
                 print m
             log_msg = _log_msg

or by just disabling E0102 there.

Anyway I think this check is good to have. As an extra safety net, in 
case pyflakes misses something.

> :E0211 (no-method-argument): *Method has no argument*
>    Used when a method which should have the bound instance as first argument has
>    no argument defined. This message belongs to the classes checker.
> *** this may be from the interfaces?
Yes, this is only found in interface definitions. This should be 
disabled locally for interfaces, as it is done in other places.
> :E0213 (no-self-argument): *Method should have "self" as first argument*
>    Used when a method has an attribute different the "self" as first argument.
>    This is considered as an error since this is a so common convention that you
>    shouldn't break it! This message belongs to the classes checker.
> *** this may be from the interfaces?
Most of this is interfaces, except one place:

https://github.com/buildbot/buildbot/blob/master/master/buildbot/test/unit/test_steps_source_svn.py#L1693

IMHO we should handle E0213 the same way as E2011.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://buildbot.net/pipermail/devel/attachments/20130711/4dbef288/attachment.html>