> > Back to everyone's favorite topic - coding style and enforcement of > > that style.
And I thought we were doing so great by leaving it behind... > > > > I'm not picking on Josef here - I'm sure I've submitted code recently > > with lint errors, this was just the review I was looking at which > > triggered the idea: > > > > https://phab.qadevel.cloud.fedoraproject.org/D389 As I already mentioned in the review, the way how Phab displays lint errors is a problem in this particular case - the diff is hard to read. That is something I'd like to avoid in the future. > > > > Since the last discussion about coding style ended in a long discussion > > about PEP8, the bits we may not like about the standard and the > > exceptions that we'd want, I'm proposing that we use strict PEP8 with > > almost no exceptions. The big hammer solution! ;-) > > I'm not saying that that PEP8 is perfect or > > that there aren't parts of it I would like to tweak but > > - it's a well known standard, easy for non-regular contributors to > > understand and use > > - style checking tools tend to use PEP8 out of the box > > - if we don't allow exceptions, there will be less time spent on > > discussing details instead of being productive :) The opposite is true for me. My experience with lint warnings in Phab (before we loosened the rules) is that I spent a considerable time rewriting and reformatting the code until the damn linter finally shut up (I'd probably need stronger words to demonstrate my temper at that time:)). Also it made the code slightly less readable sometimes (a personal view), which annoyed me. I lose no productivity on seeing an occasional lint warning in diff review (D389 is a slight exception in a very long time). That being said, I try to minimize the number of lint warnings with my every patch, unless I consider that specific rule particularly braindead. > > > > To be more specific, I am proposing the following: > > - all QA devel projects be have flake8 as the linter in arc config > > - no code be accepted with lint errors unless there is absolutely no > > other way to get around it > > - until we get our entire codebase PEP8 compliant, "if you touch a > > file, fix the lint errors even if those errors are not part of what > > you're changing" > > > > Any strong objections to starting this? Any strong objections should > > have an alternative proposal and a justification why that proposal is > > worth deviating from a well known and established standard. > > I have an alternative proposal: 1. The Phab diff should not contain too many lint warnings without a good enough reason, in order to keep it readable. 2. If it does, ask the author to fix it. 3. If the author is reluctant to change his coding style, and you have no strong objection to it per se (you just want the Phab diff to be readable), let's disable the particular lint check. In this case it would involve asking Josef to stop putting spaces between parameter keyvals, or disabling "unexpected spaces around keyword / parameter equals" check. It could also involve disabling "continuation line over-indented for hanging indent" check, because that's another very common warning we just ignore. The only problem will be when two people have strong and conflicting opinions on how to code should look. Is this the case? If not, then if we implement the solution describe above, I believe we're not going to be discussing code style in another year or two, judging by the past. > > By saying we should use strict PEP8, does that mean you want to get > rid of the linter settings we have in .arclint or leave it there? Strict PEP8 would mean all the configured exceptions would be purged. Oh boy, this will be a long discussion once again. _______________________________________________ qa-devel mailing list qa-devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/qa-devel
