5 ms·

"If someone has committed many crimes against the style guide in a PR, the reviewer should point them to the style guide..." Everything a computer can do trivi

"If someone has committed many crimes against the style guide in a PR, the reviewer should point them to the style guide..." Everything a computer can do trivially should not be done by humans. There are plenty of tools available to validate code to all the standards you can dream up. And most languages allow you to even automatically fix style errors or even enforce it for compilation (thank you Go). So instead of codifying it in a style guide document, codify it in config files for linting and autoformat tools instead. The defaults of those tools are often fine for what you need and exceptions can as easily be documented in them as well.

ISL 8y ago
> Everything a computer can do trivially should not be done by humans. Agreed, to a point. In many contexts, it is important for humans to do things themselves occasionally in order to learn how, even if a computer is generally used to do those things far more quickly.
- jelambs 8y ago
  Agreed, computers are generally way better than humans at catching many style issues. We utilize linters extensively, those enforce many of our style guide rules. However, there are some, such as the example pictured in the article, that aren’t always detectable with a linter. In these cases, we use the style guide as the source of truth to reduce the amount of personal preference that comes into code reviews.
  - aequitas 8y ago
    Agree, for everything that doesn't go automatically or would be to much effort to automate a single source of truth must be maintained.
- aequitas 8y ago
  I agree with that if you would be learning a language. But even then, being bothered by indenting, alignment, spacing etc gets tiresome and the small amounts of time lost accumulate over the day.
Coding_Cat 8y ago
Not all languages have good linting/autoformatting support (although luckilt they're rarer these days). For example: R.
- bluntfang 8y ago
  This is a discussion to be had before coding starts on a project. Do you care about style? If yes, choose a language that cares about style.
  - com2kid 8y ago
    Not always possible if the requirements of the project dictate the language.
    - wereHamster 8y ago
      Build that tool yourself if one doesn't exist yet. The whole community will thank you for it.
- zeckalpha 8y ago
  https://cran.r-project.org/web/packages/formatR/index.html https://cran.r-project.org/web/packages/formatR/index.html
  - Coding_Cat 8y ago
    We looked at it for our project, but it didn't work for us. Too little configuration & it made sections of code completely unreadable. I miss clang-format...
    - galkk 8y ago
      Google's internal Java formatter doesn't (didn't) have _any_ configuration. And, as I start to believe, it's good
ataturk 8y ago
One reason I left my previous role was because the code reviews were miserable and the team refused to automate any style matters which would have externalized and automated away many issues completely. The team claimed they were highly collaborative, but in reality, it was two heads down guys who didn't want to be bothered and would just offer niggle after niggle that had little bearing on overall quality, making the code more maintainable or ship-able. I realized they acted this way mainly because of personality deficits and likely office politics and as a cynical lever of control. There was a third developer who was genuinely decent and considerate and who had good insights to share and who was genuinely collaborative. Lastly, there was a newbie whose reviews were valuable for him to do but added no real value for me. I have worked in large and small organizations, been on projects that generated billions in revenue, and never went through what I went through there. I realized I wasn't doing my best work anymore just because I loathed the process and it wasn't going to change. Being an asshole, gas lighting, and making up policies on the fly does not help the bottom line of a company, it just pisses people off for no good reason. But hey, they are free to write the code exactly how they want and I got a $10K raise. A gift for all of us!
mnarayan01 8y ago
if a = foo() versus: if (a = foo()) Sometimes idiomatic patterns vary based on things which are not amiable to mechanical correction.
- munchbunny 8y ago
  While you have a point, I don't think the exception should disprove the rule. Even if a few specific style patterns can't be automated, the bulk can be and it's probably better to have them enforced at the pre-compile stage.
- quxbar 8y ago
  The trick is to ban even the correct but (to a human) ambiguous patterns. In this case, I wouldn't let you do assignment on the same line as the conditional.
  - squiggleblaz 8y ago
    With an `if` it's probably superfluous and therefore bannable without cost, but sometimes it can make a `while` loop so much more concise - since the alternative would be to write the same code before the loop and at the bottom of the loop. It is the cost of inexpressive languages.
- fipple 8y ago
  In what language do those produce different results?
  - mattnewton 8y ago
    In many people’s head-parser, the first one looks like an equality check (but is in fact assignment). The parens try to clue people into “this is an assignment expression!” I think, but personally I wouldn’t allow an assignment expression in an if statement even with parens.
ghostbrainalpha 8y ago
Noobie question..... Do you think there is any value in writing the code with the correct style before it is fixed in the compiler? Like is it easier to help out and spot a problem quicker, if everyone is thinking in the exact same patterns when they write code?
- btbuildem 8y ago
  If everyone is thinking in the exact same pattern, you will have terrible monoculture, and a total inability to get past certain kinds of problem. An effective team is comprised of people with a variety of backgrounds and approaches.
  - akerl_ 8y ago
    This is a very uncharitable interpretation of the parent comment. The parent comment appears to be referring to having people keep the shared style guide in mind when writing code, vs just letting the linter fix it. Extrapolating this to suggesting they don't want a team "with a variety of backgrounds and approaches" drags the conversation pretty far afield of that question.
- aequitas 8y ago
  Depends on the kind of style 'subject' I think. If it is trivial indentation, bracket placement and other preference kind of things it probably wouldn't. But if the style things lean more towards design patterns, then often there is a proper reason behind choosing that style over another and it would help because you would notice doing it the 'wrong' way and you stop to evaluate and maybe spot a bug or design issue. I think that would fall in the category of code smells like when you use a framework in a convoluted way you're probably not using the framework right or over stretching it.
  - Jach 8y ago
    I'd second this. Depending on the style in question, there could be a mismatch in thinking, but probably not for most styles. If you're used to writing very dense C code for example, transforming it at the end to meet typical Enterprise Java standards might introduce subtle logic issues (or point out existing ones) or make it harder for you to understand in the future depending on how the transformation is done, but this is really just another instance of the general refactor caution to lean on tools, incremental changes that preserve key features like function signatures, and tests to make certain you don't change behavior. Styles like forbidding single-statement if conditions that don't have curly braces helps protect you from bugs like goto-fail[0] meanwhile styles like which line your opening curly goes on, use of spacing, dare I say tabs vs spaces (outside of languages with significant whitespace anyway), and even abbrev names vs BigLongNames[1] don't really matter apart from personal preference and it being aesthetically pleasing to have local consistency. [0] https://www.imperialviolet.org/2014/02/22/applebug.html https://www.imperialviolet.org/2014/02/22/applebug.html [1] https://www.hillelwayne.com/post/the-best-se-paper/ https://www.hillelwayne.com/post/the-best-se-paper/
aequitas 8y ago
Btw, an added bonus of having a automation friendly standard for your codebase is that is can be easily converted in something you prefer reading. Prefer brackets below the condition instead of at the end? With one standard to convert to/from you can write a simple script to do that using a Git filter so every time you commit/checkout the transformation is automatically applied (https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes https://git-scm.com/book/en/v2/Customizing-Git-Git-Attribute...). Also one example on how to codify almost any style is using something like Sonarqube SSLR (https://docs.sonarqube.org/display/DEV/SSLR https://docs.sonarqube.org/display/DEV/SSLR) where the code is converted into an abstract representation where comprehensible rule can be written for (instead of for example a bunch of regexes). But you really have to be deep into wanting to enforce every aspect of a style to dive into this one.
arvinsim 8y ago
All of my formatting issues disappeared when I adopted prettier for my Javascript projects. I don't have to worry about bikeshedding/yak shaving anymore and now can focus on more important problems.

ISL 8y ago

> Everything a computer can do trivially should not be done by humans. Agreed, to a point. In many contexts, it is important for humans to do things themselves occasionally in order to learn how, even if a computer is generally used to do those things far more quickly.

jelambs 8y ago
Agreed, computers are generally way better than humans at catching many style issues. We utilize linters extensively, those enforce many of our style guide rules. However, there are some, such as the example pictured in the article, that aren’t always detectable with a linter. In these cases, we use the style guide as the source of truth to reduce the amount of personal preference that comes into code reviews.
- aequitas 8y ago
  Agree, for everything that doesn't go automatically or would be to much effort to automate a single source of truth must be maintained.
aequitas 8y ago
I agree with that if you would be learning a language. But even then, being bothered by indenting, alignment, spacing etc gets tiresome and the small amounts of time lost accumulate over the day.

Coding_Cat 8y ago

Not all languages have good linting/autoformatting support (although luckilt they're rarer these days). For example: R.

bluntfang 8y ago
This is a discussion to be had before coding starts on a project. Do you care about style? If yes, choose a language that cares about style.
- com2kid 8y ago
  Not always possible if the requirements of the project dictate the language.
  - wereHamster 8y ago
    Build that tool yourself if one doesn't exist yet. The whole community will thank you for it.
zeckalpha 8y ago
https://cran.r-project.org/web/packages/formatR/index.html https://cran.r-project.org/web/packages/formatR/index.html
- Coding_Cat 8y ago
  We looked at it for our project, but it didn't work for us. Too little configuration & it made sections of code completely unreadable. I miss clang-format...
  - galkk 8y ago
    Google's internal Java formatter doesn't (didn't) have _any_ configuration. And, as I start to believe, it's good

ataturk 8y ago

One reason I left my previous role was because the code reviews were miserable and the team refused to automate any style matters which would have externalized and automated away many issues completely. The team claimed they were highly collaborative, but in reality, it was two heads down guys who didn't want to be bothered and would just offer niggle after niggle that had little bearing on overall quality, making the code more maintainable or ship-able. I realized they acted this way mainly because of personality deficits and likely office politics and as a cynical lever of control. There was a third developer who was genuinely decent and considerate and who had good insights to share and who was genuinely collaborative. Lastly, there was a newbie whose reviews were valuable for him to do but added no real value for me. I have worked in large and small organizations, been on projects that generated billions in revenue, and never went through what I went through there. I realized I wasn't doing my best work anymore just because I loathed the process and it wasn't going to change. Being an asshole, gas lighting, and making up policies on the fly does not help the bottom line of a company, it just pisses people off for no good reason. But hey, they are free to write the code exactly how they want and I got a $10K raise. A gift for all of us!

mnarayan01 8y ago

if a = foo() versus: if (a = foo()) Sometimes idiomatic patterns vary based on things which are not amiable to mechanical correction.

munchbunny 8y ago
While you have a point, I don't think the exception should disprove the rule. Even if a few specific style patterns can't be automated, the bulk can be and it's probably better to have them enforced at the pre-compile stage.
quxbar 8y ago
The trick is to ban even the correct but (to a human) ambiguous patterns. In this case, I wouldn't let you do assignment on the same line as the conditional.
- squiggleblaz 8y ago
  With an `if` it's probably superfluous and therefore bannable without cost, but sometimes it can make a `while` loop so much more concise - since the alternative would be to write the same code before the loop and at the bottom of the loop. It is the cost of inexpressive languages.
fipple 8y ago
In what language do those produce different results?
- mattnewton 8y ago
  In many people’s head-parser, the first one looks like an equality check (but is in fact assignment). The parens try to clue people into “this is an assignment expression!” I think, but personally I wouldn’t allow an assignment expression in an if statement even with parens.

ghostbrainalpha 8y ago

Noobie question..... Do you think there is any value in writing the code with the correct style before it is fixed in the compiler? Like is it easier to help out and spot a problem quicker, if everyone is thinking in the exact same patterns when they write code?

btbuildem 8y ago
If everyone is thinking in the exact same pattern, you will have terrible monoculture, and a total inability to get past certain kinds of problem. An effective team is comprised of people with a variety of backgrounds and approaches.
- akerl_ 8y ago
  This is a very uncharitable interpretation of the parent comment. The parent comment appears to be referring to having people keep the shared style guide in mind when writing code, vs just letting the linter fix it. Extrapolating this to suggesting they don't want a team "with a variety of backgrounds and approaches" drags the conversation pretty far afield of that question.
aequitas 8y ago
Depends on the kind of style 'subject' I think. If it is trivial indentation, bracket placement and other preference kind of things it probably wouldn't. But if the style things lean more towards design patterns, then often there is a proper reason behind choosing that style over another and it would help because you would notice doing it the 'wrong' way and you stop to evaluate and maybe spot a bug or design issue. I think that would fall in the category of code smells like when you use a framework in a convoluted way you're probably not using the framework right or over stretching it.
- Jach 8y ago
  I'd second this. Depending on the style in question, there could be a mismatch in thinking, but probably not for most styles. If you're used to writing very dense C code for example, transforming it at the end to meet typical Enterprise Java standards might introduce subtle logic issues (or point out existing ones) or make it harder for you to understand in the future depending on how the transformation is done, but this is really just another instance of the general refactor caution to lean on tools, incremental changes that preserve key features like function signatures, and tests to make certain you don't change behavior. Styles like forbidding single-statement if conditions that don't have curly braces helps protect you from bugs like goto-fail[0] meanwhile styles like which line your opening curly goes on, use of spacing, dare I say tabs vs spaces (outside of languages with significant whitespace anyway), and even abbrev names vs BigLongNames[1] don't really matter apart from personal preference and it being aesthetically pleasing to have local consistency. [0] https://www.imperialviolet.org/2014/02/22/applebug.html https://www.imperialviolet.org/2014/02/22/applebug.html [1] https://www.hillelwayne.com/post/the-best-se-paper/ https://www.hillelwayne.com/post/the-best-se-paper/

aequitas 8y ago

Btw, an added bonus of having a automation friendly standard for your codebase is that is can be easily converted in something you prefer reading. Prefer brackets below the condition instead of at the end? With one standard to convert to/from you can write a simple script to do that using a Git filter so every time you commit/checkout the transformation is automatically applied (https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes https://git-scm.com/book/en/v2/Customizing-Git-Git-Attribute...). Also one example on how to codify almost any style is using something like Sonarqube SSLR (https://docs.sonarqube.org/display/DEV/SSLR https://docs.sonarqube.org/display/DEV/SSLR) where the code is converted into an abstract representation where comprehensible rule can be written for (instead of for example a bunch of regexes). But you really have to be deep into wanting to enforce every aspect of a style to dive into this one.

arvinsim 8y ago

All of my formatting issues disappeared when I adopted prettier for my Javascript projects. I don't have to worry about bikeshedding/yak shaving anymore and now can focus on more important problems.