5 ms·

Honestly for me it tells me they need to hook up a code auto formatter into their workflow Forget stylistic nitpicking. Enforce a code quality standard with a

Honestly for me it tells me they need to hook up a code auto formatter into their workflow Forget stylistic nitpicking. Enforce a code quality standard with a linter and formatter and be done with it

barbazoo 2y ago
Oh 100%. Discussing style within a PR unrelated to changing the styling rules seems like a waste of time.
deleted 2y ago
[deleted]
DanHulton 2y ago
This literally what the article says.
InvaderFizz 2y ago
I agree. Even within my own org there is some nitpicking, but it's almost always about style consistency for our shared codebase, which is valid. If your PR doesn't pass lint checks, it doesn't get merged. And the only reason it would fail the lint checks is if your pre-commit hooks didn't fire. There is no argument of 2,4,8 space vs tabs, because the code you commit is run through the linter. Write however you want for the things that don't matter, the formatter always wins.
chuckhend 2y ago
Automating this with linter and formatter is great. It moves the argument over style and format to a one liner change to a lint config instead of mingling it with the with the main code change.
- bluefirebrand 2y ago
  It also hopefully only happens once. If you continue to have people bringing up arguments over the linter and formatter after an initial agreement is made, then you can talk to those people
edflsafoiewq 2y ago
Style also involves things like "renaming variables", to take the article's example, which can't be automated away.
- AlotOfReading 2y ago
  There are formatters that will enforce the basic case/underscore rules of naming conventions. I haven't seen one for the actual text, but I can see how that might be nice if it worked reliably.
marcosdumay 2y ago
This. You either don't care about style or push the style automatically without any interaction. Every other option is bad to some extent. But only use a linter if you will add your rules over the empty set. If you get a pre-built set and are expected to remove the ones you don't need, you are making a toxic environment.
arp242 2y ago
It's impossible for any code formatter to be 100%. Where to put a blank line? Where to break a line? How to name a variable/function? etc. etc. Some try (e.g. prettier), and what you end up is frankly just bizarre code that's just ugly. Never mind tons of other small things that often don't really matter. The solution is to just not be too anal about it. It really is a cultural problem. For example a few weeks ago I reviewed a PR from a new team member; there were some seriously structural problems with his approach, so I commented on that and ignored all the small stuff for now. Another programmer on my team also reviewed at the same time, and only commented on the small stuff that, IMHO, don't really matter, and didn't look at the general approach at all (which really was just all wrong, and also quite obviously wrong). Not to be too arrogant about it, but I feel this sort of stuff is what distinguishes a "good engineer" from a "mid engineer".
- bluefirebrand 2y ago
  > The solution is to just not be too anal about it. It really is a cultural problem "Any proposal that requires everyone to just is not a solution, because everyone will not just" People are anal. You aren't going to get them to stop being anal A real solution is to have the team agree on a shared style guide, then enforce it with a linter and formatter. If anyone cannot come to an agreement with the rest of the team, or continues to be anal about things that do not appear in the style guide after this, then that person has singled themselves out and the company will need to find a way to deal with the behavior I agree that good engineers focus more on the actual structure and problems instead of nitpicky things like formatting That said, code cleanliness and consistency is important too. It makes codebases much easier to maintain and understand if everything is formatter consistently. It's a pretty mid engineer take to think it's not important at all
  - arp242 2y ago
    You can't "enforce" all of these things. That was my main point. And you absolutely can stop people from doing that. Simply accepting dickish asshole toxic behaviour as "well, people are like that shrug" and never telling people off is exactly the problem.
chipdart 2y ago
> Enforce a code quality standard with a linter and formatter and be done with it Adopting a linter does not eliminate style issues. It just eliminates the number of possible comments a PR can get over style issues by replacing all detailed feedback with just one comment with a very clear actionable request that's trivial to satisfy: "It seems X doesn't look right. Could you please run the linter?"

Honestly for me it tells me they need to hook up a code auto formatter into their workflow Forget stylistic nitpicking. Enforce a code quality standard with a linter and formatter and be done with it

barbazoo 2y ago

Oh 100%. Discussing style within a PR unrelated to changing the styling rules seems like a waste of time.

deleted 2y ago

[deleted]

DanHulton 2y ago

This literally what the article says.

InvaderFizz 2y ago

I agree. Even within my own org there is some nitpicking, but it's almost always about style consistency for our shared codebase, which is valid. If your PR doesn't pass lint checks, it doesn't get merged. And the only reason it would fail the lint checks is if your pre-commit hooks didn't fire. There is no argument of 2,4,8 space vs tabs, because the code you commit is run through the linter. Write however you want for the things that don't matter, the formatter always wins.

chuckhend 2y ago

Automating this with linter and formatter is great. It moves the argument over style and format to a one liner change to a lint config instead of mingling it with the with the main code change.

bluefirebrand 2y ago
It also hopefully only happens once. If you continue to have people bringing up arguments over the linter and formatter after an initial agreement is made, then you can talk to those people

edflsafoiewq 2y ago

Style also involves things like "renaming variables", to take the article's example, which can't be automated away.

AlotOfReading 2y ago
There are formatters that will enforce the basic case/underscore rules of naming conventions. I haven't seen one for the actual text, but I can see how that might be nice if it worked reliably.

marcosdumay 2y ago

This. You either don't care about style or push the style automatically without any interaction. Every other option is bad to some extent. But only use a linter if you will add your rules over the empty set. If you get a pre-built set and are expected to remove the ones you don't need, you are making a toxic environment.

arp242 2y ago

It's impossible for any code formatter to be 100%. Where to put a blank line? Where to break a line? How to name a variable/function? etc. etc. Some try (e.g. prettier), and what you end up is frankly just bizarre code that's just ugly. Never mind tons of other small things that often don't really matter. The solution is to just not be too anal about it. It really is a cultural problem. For example a few weeks ago I reviewed a PR from a new team member; there were some seriously structural problems with his approach, so I commented on that and ignored all the small stuff for now. Another programmer on my team also reviewed at the same time, and only commented on the small stuff that, IMHO, don't really matter, and didn't look at the general approach at all (which really was just all wrong, and also quite obviously wrong). Not to be too arrogant about it, but I feel this sort of stuff is what distinguishes a "good engineer" from a "mid engineer".

bluefirebrand 2y ago
> The solution is to just not be too anal about it. It really is a cultural problem "Any proposal that requires everyone to just is not a solution, because everyone will not just" People are anal. You aren't going to get them to stop being anal A real solution is to have the team agree on a shared style guide, then enforce it with a linter and formatter. If anyone cannot come to an agreement with the rest of the team, or continues to be anal about things that do not appear in the style guide after this, then that person has singled themselves out and the company will need to find a way to deal with the behavior I agree that good engineers focus more on the actual structure and problems instead of nitpicky things like formatting That said, code cleanliness and consistency is important too. It makes codebases much easier to maintain and understand if everything is formatter consistently. It's a pretty mid engineer take to think it's not important at all
- arp242 2y ago
  You can't "enforce" all of these things. That was my main point. And you absolutely can stop people from doing that. Simply accepting dickish asshole toxic behaviour as "well, people are like that shrug" and never telling people off is exactly the problem.

chipdart 2y ago

> Enforce a code quality standard with a linter and formatter and be done with it Adopting a linter does not eliminate style issues. It just eliminates the number of possible comments a PR can get over style issues by replacing all detailed feedback with just one comment with a very clear actionable request that's trivial to satisfy: "It seems X doesn't look right. Could you please run the linter?"