Like the pep8 check, this will elevate the quality of all future patches.
I disagree. The pep8 checks (especially the ones we have enabled) enforce basic consistency and basic mechanics. Requiring every tiny and obvious utility function to have a docstring is not equivalent to quality. Writing functions that are small in scope and clearly-readable is, IMHO. Except in a public interface, the latter obviates the need for the former in almost ever case.
Clearly, that requires all of the pylint checks to pass for a given file, or the continuous integration check will fail.
Maybe this will help: I really have no intention of making passing pylint (as configured) a requirement for all patches.
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.
I care far more about people that write drivers, add features to drivers, or fix bugs in drivers. A lot of those contributions come from people that aren't going to tolerate a high level of obsession over things that appear trivial to them. Since I'm not going to bring myself to reject their very useful and functional patch because of such a missing thing, if I enforce it before commit, then I have to do the work to fix them up each time.
Further, I don't want to elevate the bar so high that it appears to not be worth even trying.
I appreciate what you're trying to do, and in an ideal world, we'd completely pass all static analysis everywhere all the time. I just don't think it's appropriate for a project like this.
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.
Since I don't think that "useful" documentation is needed on each of the placeholder-documented things, I don't think I'm going to change my mind about the need to enforce them until I see them.
Note that I'm not saying "show me them and I'll be convinced." I'm saying I agree that these have little value. If I thought the end goal was extremely worthwhile, then the iterative approach to getting us there would make sense.
Instead, we should embrace iterative and incremental development.
Can we add pragmatic to the above list? Start with a list of pylint rules that we can all agree on and move that forward. The changes like the ones that avoid conflicts with reserved words are reasonable. Let's make those fixes and get pylint gating on those. Then we can discuss/argue about raising the bar on specific details and consider each on its own merits.
--Dan