11 ms·

> And yet, I see a whole swath of the industry hyper-focused on various linters/styling/rules. It seems to me that what you're actually seeing is an entire ind

> And yet, I see a whole swath of the industry hyper-focused on various linters/styling/rules. It seems to me that what you're actually seeing is an entire industry trying to eliminate all code-related issues, specially bike-shedding ones. This is patently obvious to anyone who was forced to waste their time in code review iterations discussing, say, where a brace should go and how many spaces someone should have added.

s17n 6y ago
Nobody was ever "forced to waste their time" on this stuff. I have a simple rule - I don't comment on other people's style, and if people comment on my style, I just go with their suggestions. Problem solved.
- ses1984 6y ago
  You've never been on a team with two people with opposing opinions I guess.
  - Quekid5 6y ago
    Just out of morbid curiosity... have you actually experienced multiple 'seniors' giving conflicting code review comments about code style (of all things)? That sounds quite dysfunctional. (EDIT: Sure, nitpicks may differ, but...)
    - ses1984 6y ago
      Style yes, formatting no. Worked with two developers who endlessly argued whether or not we should handle a certain bit of complexity in a certain layer or the next layer over, so we ended up handling it in both layers with the downsides of both.
    - rumanator 6y ago
      > That sounds quite dysfunctional. No,it doesn't. It sounds like the expected outcome of not enforcing an established style with automated tools. All it takes is someone posting a merge request with a bracket out of place, or tabs instead of spaces which screws layout because yes IDEs have custom definitions and a dude happened to have opened a source file with an editor that wasn't properly configured. Boom, merge request receives two comments pointing out the bracket and how indentation is off. Congrats, about 20 minutes of your team's day are wasted because that's the time it takes to receive feedback from the merge request, be briefed on the remarks, go through the code and fix whitespaces, commit your change, push those changes, update the merge request, and wait for a team member to review your update. No drama. No dysfunctional team. No disagreement, even. But those 20 minutes of your life are lost forever.
      - Quekid5 6y ago
        > It sounds like the expected outcome of not enforcing an established style with automated tools. Unfortunately those also have significant downsides around large-scale refactoring. What I always do (and advise other code reviewers to do) is to just ask themselves: Does this code follow the local style in the file being edited? That simplifies things greatly, IME.
- johnisgood 6y ago
  > I just go with their suggestions. Why though? I am not going to go with suggestions if they make the code less readable for me!
  - codetrotter 6y ago
    I think the whole world would benefit if we’d make it so that code formatting happened separately from what was committed. Then everyone could have their local checkouts formatted the way they wanted and there would be nothing to argue about in terms of coding style.
    - markrages 6y ago
      Smart tabs is halfway there.
    - brigandish 6y ago
      This would be my number one wish for tooling.
    - goto11 6y ago
      Why though? It seems like an elaborate technical solution just to avoid someone making a decision on fruitless bike-shedding discussions. It will only work for white space formatting not for other conventions prone to bikeshedding like naming/casing conventions. Just make decision and get on with your lives. Have some kind of linter check for inconsistencies before any human review. If an organization cannot make a decision on inconsequential bike-shedding it is dysfunctional.
    - frandroid 6y ago
      After pre-commit linting, you get post-pull linting... If you kept the code in a RAM drive this might even be quick. 8)
- lallysingh 6y ago
  Every have more than one reviewer in your CR?
- mrighele 6y ago
  Having different prople using different coding styles adds a lot of noise to the history of you repo, expecially if those differences are not only about whitespace (e.g. one developer insisting on opening bra kets on the same line and another on a new line)
Osiris 6y ago
I completely agree. One reason I like prettier is that it only has about 6 options you can change. It removes the bike-shedding. Just let it do it's thing and worry about more important things. It also remove all debate in PRs about style and formatting. (note: before prettier, I was fairly particular about how I formatted my code, and I disagreed with prettier in some cases, but now, I love having one less thing to think about)
Quekid5 6y ago
This usually a good time to apply the When In Rome rule. Do not reformat needlessly, follow the code style of the code you're modifying. Done. (If multiple people are arguing back and forth in code review -- when following the WIR rule -- tell them about the WIR rule and that should settle it. If not, you have bigger problems in your team.)
- jacobsenscott 6y ago
  Sort of. You shouldn't combine style and non-style changes in on change set. But if your project has mixed styles from file to file you should assimilate the locals over time, just as the Romans did. A consistent style has value.
  - Quekid5 6y ago
    I agree (in principle!) with the idea that consistency across the whole code base has value, but personally I find it extremely marginal, unless you're literally dealing with e.g. hundreds of people needing to read/modify the code. The value proposition skews heavily towards situations where you have a lot of people needing to read/modify the code.
lokar 6y ago
This. The point of the style guide, now reaching its best embodiment in clang-format, gofmt, and others, is that you don't want to waste time arguing about, or even considering for a moment the formatting of anything.
- mgkimsal 6y ago
  the people who might not follow explicit guides are also the ones who don't typically spend the time 'arguing' about it in the first place though.
Aeolun 6y ago
Doing any kind of style discussion in a code review means you’ve already failed. I personally get super annoyed when people keep pointing out style issues, but our CI tool can notify me of issues with my commit until the end of time without me getting frustrated with it.
- rumanator 6y ago
  > Doing any kind of style discussion in a code review means you’ve already failed This sort of baseless assertion has no bearing in reality. In a project that hasn't adopted any linting tools and automatic style checks, all it takes is a misconfigured editor to post a change request that fails to comply with style guides. These sorts of absolutes show a complete detachment from reality and absence of any practical experience in the field.
  - Aeolun 6y ago
    > These sorts of absolutes show a complete detachment from reality and absence of any practical experience in the field. But you are making these baseless assertions yourself? Obviously you can have issues if you are not using automated linting (both in the editor and on CI). That’s part of the failure.

s17n 6y ago

Nobody was ever "forced to waste their time" on this stuff. I have a simple rule - I don't comment on other people's style, and if people comment on my style, I just go with their suggestions. Problem solved.

ses1984 6y ago
You've never been on a team with two people with opposing opinions I guess.
- Quekid5 6y ago
  Just out of morbid curiosity... have you actually experienced multiple 'seniors' giving conflicting code review comments about code style (of all things)? That sounds quite dysfunctional. (EDIT: Sure, nitpicks may differ, but...)
  - ses1984 6y ago
    Style yes, formatting no. Worked with two developers who endlessly argued whether or not we should handle a certain bit of complexity in a certain layer or the next layer over, so we ended up handling it in both layers with the downsides of both.
  - rumanator 6y ago
    > That sounds quite dysfunctional. No,it doesn't. It sounds like the expected outcome of not enforcing an established style with automated tools. All it takes is someone posting a merge request with a bracket out of place, or tabs instead of spaces which screws layout because yes IDEs have custom definitions and a dude happened to have opened a source file with an editor that wasn't properly configured. Boom, merge request receives two comments pointing out the bracket and how indentation is off. Congrats, about 20 minutes of your team's day are wasted because that's the time it takes to receive feedback from the merge request, be briefed on the remarks, go through the code and fix whitespaces, commit your change, push those changes, update the merge request, and wait for a team member to review your update. No drama. No dysfunctional team. No disagreement, even. But those 20 minutes of your life are lost forever.
    - Quekid5 6y ago
      > It sounds like the expected outcome of not enforcing an established style with automated tools. Unfortunately those also have significant downsides around large-scale refactoring. What I always do (and advise other code reviewers to do) is to just ask themselves: Does this code follow the local style in the file being edited? That simplifies things greatly, IME.
johnisgood 6y ago
> I just go with their suggestions. Why though? I am not going to go with suggestions if they make the code less readable for me!
- codetrotter 6y ago
  I think the whole world would benefit if we’d make it so that code formatting happened separately from what was committed. Then everyone could have their local checkouts formatted the way they wanted and there would be nothing to argue about in terms of coding style.
  - markrages 6y ago
    Smart tabs is halfway there.
  - brigandish 6y ago
    This would be my number one wish for tooling.
  - goto11 6y ago
    Why though? It seems like an elaborate technical solution just to avoid someone making a decision on fruitless bike-shedding discussions. It will only work for white space formatting not for other conventions prone to bikeshedding like naming/casing conventions. Just make decision and get on with your lives. Have some kind of linter check for inconsistencies before any human review. If an organization cannot make a decision on inconsequential bike-shedding it is dysfunctional.
  - frandroid 6y ago
    After pre-commit linting, you get post-pull linting... If you kept the code in a RAM drive this might even be quick. 8)
lallysingh 6y ago
Every have more than one reviewer in your CR?
mrighele 6y ago
Having different prople using different coding styles adds a lot of noise to the history of you repo, expecially if those differences are not only about whitespace (e.g. one developer insisting on opening bra kets on the same line and another on a new line)

Osiris 6y ago

I completely agree. One reason I like prettier is that it only has about 6 options you can change. It removes the bike-shedding. Just let it do it's thing and worry about more important things. It also remove all debate in PRs about style and formatting. (note: before prettier, I was fairly particular about how I formatted my code, and I disagreed with prettier in some cases, but now, I love having one less thing to think about)

Quekid5 6y ago

This usually a good time to apply the When In Rome rule. Do not reformat needlessly, follow the code style of the code you're modifying. Done. (If multiple people are arguing back and forth in code review -- when following the WIR rule -- tell them about the WIR rule and that should settle it. If not, you have bigger problems in your team.)

jacobsenscott 6y ago
Sort of. You shouldn't combine style and non-style changes in on change set. But if your project has mixed styles from file to file you should assimilate the locals over time, just as the Romans did. A consistent style has value.
- Quekid5 6y ago
  I agree (in principle!) with the idea that consistency across the whole code base has value, but personally I find it extremely marginal, unless you're literally dealing with e.g. hundreds of people needing to read/modify the code. The value proposition skews heavily towards situations where you have a lot of people needing to read/modify the code.

lokar 6y ago

This. The point of the style guide, now reaching its best embodiment in clang-format, gofmt, and others, is that you don't want to waste time arguing about, or even considering for a moment the formatting of anything.

mgkimsal 6y ago
the people who might not follow explicit guides are also the ones who don't typically spend the time 'arguing' about it in the first place though.

Aeolun 6y ago

Doing any kind of style discussion in a code review means you’ve already failed. I personally get super annoyed when people keep pointing out style issues, but our CI tool can notify me of issues with my commit until the end of time without me getting frustrated with it.

rumanator 6y ago
> Doing any kind of style discussion in a code review means you’ve already failed This sort of baseless assertion has no bearing in reality. In a project that hasn't adopted any linting tools and automatic style checks, all it takes is a misconfigured editor to post a change request that fails to comply with style guides. These sorts of absolutes show a complete detachment from reality and absence of any practical experience in the field.
- Aeolun 6y ago
  > These sorts of absolutes show a complete detachment from reality and absence of any practical experience in the field. But you are making these baseless assertions yourself? Obviously you can have issues if you are not using automated linting (both in the editor and on CI). That’s part of the failure.