[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