[chirp_devel] [PATCH 06/11] Fix pylint issues in __init__.py files (#159)

Zach Welch
Tue Mar 10 22:07:40 PDT 2015


On 03/10/2015 08:45 PM, Tom Hayward wrote:
> On Mon, Mar 9, 2015 at 7:40 AM, Zach Welch <zach at mandolincreekfarm.com> wrote:
>> I agree that many of the module-level docstrings that I added need
>> improvement, but their presence serves as a reminder to add more
>> information down the road.
>
> A much better reminder that a docstring needs improvement is a warning
> from pylint. It looks like what you've done is provided a low-quality
> placeholder just to shut up pylint. This kind of thing defeats the
> purpose of pylint. I'm failing to see the value in this change.

That's a valid observation, but it drives at the more fundamental reason 
that I was working on this issue.

At the very start, I added pylint to the automatic checking script, 
thinking that it would be good to have it run as part of a continuous 
integration script.  Like the pep8 check, this will elevate the quality 
of all future patches.  Clearly, that requires all of the pylint checks 
to pass for a given file, or the continuous integration check will fail.

Without fixing every issue, it can't be used continuously.  If it is not 
part of a standard and automated review/commit process, then it will not 
be used universally by developers, so it might as well not be used at all.

Without enabling all of the checks that we want enforced, we cannot 
expect new code to achieve that standard, as it will not be caught by 
the integration scripts.  Sure, not every check needs to be turned on, 
so the broader question here is what standards do the CHIRP developers 
care to enforce.  Obviously, I think docstrings should be mandatory, 
thus I added a placeholder.

Right now, it seems clear that no one has been running pylint at all, so 
its purpose -- to be used -- has already been defeated.  As such, I 
think that getting the tree to a point that it runs cleanly -- with all 
of the checks that should be enabled -- does have clear and present 
value.  In the context of improving the continuous integration process, 
a placeholder is tangibly better than nothing at all.  Without that 
ulterior motive, I would agree that the change has no value.

Now, take another step back for even more perspective.  I would be happy 
to write suitable documentation to replace the placeholder, but I think 
it would be unreasonable to block these patches until I do that. 
Instead, we should embrace iterative and incremental development. 
Specifically, I need time to grok things sufficiently in order to write 
good documentation.  In the meantime, I will do what I can to move 
things forward, and I think automating pylint moves us forward.
-- 
Zach Welch
Mandolin Creek Farm
www.mandolincreekfarm.com
farm: 541-453-4131
cell: 541-740-3410



More information about the chirp_devel mailing list