03-27-2016, 01:48 AM
(This post was last modified: 03-27-2016, 01:57 AM by LogicParrot.)
Quote:I'm not saying it was awful. Just the general no-reaction whatsoever interspaced with one comment about one line... i get enough of that from my day job and having to adhere to yet another set of arbitrary rules (5 spaces between functions?! really? so that you could never see two functions and never understand any links between?
I can see your point. I often see perfectly good PRs that receive several comments, where almost all of the comments are style related, and not logic-related. While I believe style guidelines are very important, I can see how this can be energy-sapping, and can give the impression that "these devs are nitpicky and only care about eye-candy, and I think they did not even review my code logic, they just hunt for my style problems". Sometimes this is the feeling I had when I started contributing. While it did not really bother me, I can see how it can bother others.
One middle-ground solution which does not involve relaxing the style would be not mentioning a PR's style issues until all logic problems are commented upon, and all non-style code issues are fixed. Once the crux of the PR is declared good, style comments are to be made. One could establish a simple rule: "Never comment on style problems unless one of the devs declares this PR's logic good.". This might help dissipate the perceived "no" atmosphere that the style reviews inevitably create, and each good PR would receive at least one positive "looks-good" comment even if it has style issues.
Of course, edge cases where the style is absolutely horrible and makes the eyes bleed should not be logic-reviewed before the code is put into at least a readable state. And in this case ranting about the style before checking the logic is acceptable.
This might be completely unnecessary and/or ineffective, I am just thinking out loud.
Edit: As a case in point, my latest PR just got a style comment. This is a very typical occurrence. While this is a perfectly valid style comment, I can imagine how this can negatively affect a newbie, especially when said newbie has submitted far heavier changes and many such comments are received prior to anyone doing code-logic review.