5 ms·

This is why I love things like cargo fmt / go fmt / eslint / etc. No discussions of whether if (foo) return; is valid, or we should do if (foo) {

This is why I love things like cargo fmt / go fmt / eslint / etc. No discussions of whether if (foo) return; is valid, or we should do if (foo) { return; }

hibikir 2y ago
That's also why I dislike it: The mind that will go apeshit over trivial style nonsense will still provide unhelpful feedback on other topics... but it will become far less obvious that their comments should be downgraded. Formatters also do a poor job if your language is flexible and expressive. Great for go, but if your language is the kind that easily supports internal DSLs, formatters are not even helpful at making the code regular
- Jare 2y ago
  > it will become far less obvious that their comments should be downgraded. I'm curious why you think this would happen... I would imagine that their comments will now have be to about things that matter, and if they are unhelpful they will stand out more.
  - Brian_K_White 2y ago
    It is obvious that a style comment is of no significance, and that the person who made it chose to spend time and personal capital on something of no significance. It is not obvious that an actual code change is of poor quality. To show that takes infinitely more experience, work to analyse all the direct and indirect ramifications of both the original and proposed approaches, capacity to push back and decide that your own engineering judgement is equal or better than whoever is purporting to correct you, willingness to suffer everyone else accusing you of arrogance for that on top. Even with all of that, it's a lot harder to prove the valueless comment is valueless because the more you know, the more you know that practically every single line of code could be done 30 other ways with some valid argument for each one. It's completely insidious both for the junior and the senior. The junior has no way to know the junk comment is junk. So they internalize the comment and everyone is worse for it. The senior has it almost worse. They know enough to know they don't know everything and the comment might be valid, so they put in all kinds of work to try to figure out if they actually missed something, did they have point etc. Maybe in the end the code doesn't end up as bad as the junior just rolling over, but it sucked for everyone and the challenge was not really an intentional productive crucible, it was just a douche that everyone had to waste time and energy taking seriously. Only someone who actually is arrogant (whatever level, whichever side of the pr) has it easy. Again bad for everyone except them maybe.
    - 1123581321 2y ago
      If the seniors are too passive to say no faster and faster, and management is absent, the junior’s going to end up being the tech lead. I’ve seen this. :) It’s not necessarily even bad if there is no meaningful leadership above or alongside the junior. The fast-rising junior may end up hiring developers who are more assertive and still capable.
- chipdart 2y ago
  > over trivial style nonsense What's your definition of "trivial style nonsense"? Is it "something I personally don't understand or care"? Things like tabs vs spaces is important, and so is how many spaces an indentation level should take. This affects how editors reformat code, how other people's editors present the code, and even how many lines a commit has and is blamed for a change. That's why linters are of critical importance to a team, and so is adopting a shared standard and editor config. The only people who don't understand the importance of a style guide and how to make it a non-issue by enforcing tools to enforce it are those with little to no experience working with software.
  - avidiax 2y ago
    I have lots of experience working with software. Linting and style guides are not of "critical importance". No business objective will go unachieved because some checkins use tabs and others use spaces. Whatever problem style issues might cause can be resolved before lunch by running a formatter and committing the result.
    - emptiestplace 2y ago
      > resolved before lunch blame.ignoreRevsFile
    - chipdart 2y ago
      > Linting and style guides are not of "critical importance". This is simply false, as attested by the huge volume of comments in this thread by those with actual professional experience working on real-world software projects. You're also oblivious to the problem domain, because otherwise you'd understand that the critical problems are not whether a space should be at the left or at the right of a symbol, but all the churn that is required to manually address style problems in PRs. Try to think about the problem. You post a PR that screws up all formatting. It takes time for a team member to review a PR. Once you start to get reviews,you notice comments pointing out failures in complying with a specific style. Whether you go the passive-aggressive path of waiting for any other team member to review your code or you do the right thing and fix the problems you introduced, that requires another round of PR reviews. The time that you take with each iteration is the time your work is delayed to be merged. Now think about how many hours per month you waste just because you can't manage to format your code properly.
- philwelch 2y ago
  Can you give an example of what you mean by a language supporting “internal DSL’s”?
  - Jare 2y ago
    Scala would be my immediate thought. Probably also C/C++ with macros.
mihaaly 2y ago
Who cares?!
- meowfly 2y ago
  I agree. I can read both of them clearly. I'm not sure what value we are maximizing for in this example. It's definitely not readability.
  - climb_stealth 2y ago
    Bugs. Subtle horrible bugs that can take forever to find. Not having the braces makes it so easy to accidentally break in later changes or refactoring. Sure on its own it's super obvious, but next to other changes or refactoring it is really easy to miss that suddenly logic falls out of the condition or doesn't make sense anymore. I don't remember the details but I've had some typo in a one-line-if-condition once and it took me days to find it. Might have been an accidental semicolon in C. A linter enforced the braces and line-breaks and made it obvious. Anyways, running a linter also helps because all anger or frustration can be directed towards the machine.
    - Timwi 2y ago
      > Not having the braces makes it so easy to accidentally break in later changes or refactoring. Did you mean “not having a linter”? Because once you have a linter you no longer need the braces because the autoformatting will always fix the indentation.
      - climb_stealth 2y ago
        I was thinking of a case like this: if (foo) return; Perfectly valid and people do it. But then later on someone might put a debug statement above the return to check something. Or add some other logic. if (foo) doSomething(); return; And suddenly the logic is broken and the return falls out of the if-condition. It looks fairly contrived with an example like this but easy to do when code is a bit more complicated or people are tired or rushed. It's a good habit to always enforce braces for if-conditions as it defeats that whole category of mistakes. Fairly innocent but can be so hard to find because of that. A linter would enforce having braces and solve the issue. And for codebases without a linter it is a good habit to just go with the braces.
theshrike79 2y ago
Just having a valid .editorconfig for the project will do a lot for languages without gofmt.
chipdart 2y ago
> This is why I love things like cargo fmt / go fmt / eslint / etc. I agree. Once I had the displeasure of working with a junior dev who was very prolific in posting comments on style and if a space should be at the left or at the right of a symbol. It took me a few days of dealing with that noise to onboard a linter. Even so the junior dev felt entitled to manifest how high their standards were by posting a torrent of comments in the PR that onboarded the team's official linter config. But once the PR was approved and merged, surprise surprise: the junior dev's PR comment metrics dropped from dozens per PR to zero. The style guide didn't even had to be enforced. The last I've heard about the junior dev was throwing a tantrum when one of their PRs received a comment from another team member asking to run the linter because it failed to adhere to the team's official style guide. Apparently the high standards and this attention to detail only went one way.
- plasticchris 2y ago
  That’s why you hook the linter up so it runs on every commit / push. No need to ask, it always runs. And no need to quibble over style. Don’t like it? Change the linter.
  - Aeolun 2y ago
    I very much dislike any process modifying my commit after I submit it. Rebasing becomes hell.
    - plasticchris 2y ago
      run it in a hook that runs before the commit is made
      - cempaka 2y ago
        Yeah we just have any linter failures fail the CI build, but it's left up to submitters to decide how to resolve the violations.
        2y ago

This is why I love things like cargo fmt / go fmt / eslint / etc. No discussions of whether if (foo) return; is valid, or we should do if (foo) { return; }

hibikir 2y ago

That's also why I dislike it: The mind that will go apeshit over trivial style nonsense will still provide unhelpful feedback on other topics... but it will become far less obvious that their comments should be downgraded. Formatters also do a poor job if your language is flexible and expressive. Great for go, but if your language is the kind that easily supports internal DSLs, formatters are not even helpful at making the code regular

Jare 2y ago
> it will become far less obvious that their comments should be downgraded. I'm curious why you think this would happen... I would imagine that their comments will now have be to about things that matter, and if they are unhelpful they will stand out more.
- Brian_K_White 2y ago
  It is obvious that a style comment is of no significance, and that the person who made it chose to spend time and personal capital on something of no significance. It is not obvious that an actual code change is of poor quality. To show that takes infinitely more experience, work to analyse all the direct and indirect ramifications of both the original and proposed approaches, capacity to push back and decide that your own engineering judgement is equal or better than whoever is purporting to correct you, willingness to suffer everyone else accusing you of arrogance for that on top. Even with all of that, it's a lot harder to prove the valueless comment is valueless because the more you know, the more you know that practically every single line of code could be done 30 other ways with some valid argument for each one. It's completely insidious both for the junior and the senior. The junior has no way to know the junk comment is junk. So they internalize the comment and everyone is worse for it. The senior has it almost worse. They know enough to know they don't know everything and the comment might be valid, so they put in all kinds of work to try to figure out if they actually missed something, did they have point etc. Maybe in the end the code doesn't end up as bad as the junior just rolling over, but it sucked for everyone and the challenge was not really an intentional productive crucible, it was just a douche that everyone had to waste time and energy taking seriously. Only someone who actually is arrogant (whatever level, whichever side of the pr) has it easy. Again bad for everyone except them maybe.
  - 1123581321 2y ago
    If the seniors are too passive to say no faster and faster, and management is absent, the junior’s going to end up being the tech lead. I’ve seen this. :) It’s not necessarily even bad if there is no meaningful leadership above or alongside the junior. The fast-rising junior may end up hiring developers who are more assertive and still capable.
chipdart 2y ago
> over trivial style nonsense What's your definition of "trivial style nonsense"? Is it "something I personally don't understand or care"? Things like tabs vs spaces is important, and so is how many spaces an indentation level should take. This affects how editors reformat code, how other people's editors present the code, and even how many lines a commit has and is blamed for a change. That's why linters are of critical importance to a team, and so is adopting a shared standard and editor config. The only people who don't understand the importance of a style guide and how to make it a non-issue by enforcing tools to enforce it are those with little to no experience working with software.
- avidiax 2y ago
  I have lots of experience working with software. Linting and style guides are not of "critical importance". No business objective will go unachieved because some checkins use tabs and others use spaces. Whatever problem style issues might cause can be resolved before lunch by running a formatter and committing the result.
  - emptiestplace 2y ago
    > resolved before lunch blame.ignoreRevsFile
  - chipdart 2y ago
    > Linting and style guides are not of "critical importance". This is simply false, as attested by the huge volume of comments in this thread by those with actual professional experience working on real-world software projects. You're also oblivious to the problem domain, because otherwise you'd understand that the critical problems are not whether a space should be at the left or at the right of a symbol, but all the churn that is required to manually address style problems in PRs. Try to think about the problem. You post a PR that screws up all formatting. It takes time for a team member to review a PR. Once you start to get reviews,you notice comments pointing out failures in complying with a specific style. Whether you go the passive-aggressive path of waiting for any other team member to review your code or you do the right thing and fix the problems you introduced, that requires another round of PR reviews. The time that you take with each iteration is the time your work is delayed to be merged. Now think about how many hours per month you waste just because you can't manage to format your code properly.
philwelch 2y ago
Can you give an example of what you mean by a language supporting “internal DSL’s”?
- Jare 2y ago
  Scala would be my immediate thought. Probably also C/C++ with macros.

mihaaly 2y ago

Who cares?!

meowfly 2y ago
I agree. I can read both of them clearly. I'm not sure what value we are maximizing for in this example. It's definitely not readability.
- climb_stealth 2y ago
  Bugs. Subtle horrible bugs that can take forever to find. Not having the braces makes it so easy to accidentally break in later changes or refactoring. Sure on its own it's super obvious, but next to other changes or refactoring it is really easy to miss that suddenly logic falls out of the condition or doesn't make sense anymore. I don't remember the details but I've had some typo in a one-line-if-condition once and it took me days to find it. Might have been an accidental semicolon in C. A linter enforced the braces and line-breaks and made it obvious. Anyways, running a linter also helps because all anger or frustration can be directed towards the machine.
  - Timwi 2y ago
    > Not having the braces makes it so easy to accidentally break in later changes or refactoring. Did you mean “not having a linter”? Because once you have a linter you no longer need the braces because the autoformatting will always fix the indentation.
    - climb_stealth 2y ago
      I was thinking of a case like this: if (foo) return; Perfectly valid and people do it. But then later on someone might put a debug statement above the return to check something. Or add some other logic. if (foo) doSomething(); return; And suddenly the logic is broken and the return falls out of the if-condition. It looks fairly contrived with an example like this but easy to do when code is a bit more complicated or people are tired or rushed. It's a good habit to always enforce braces for if-conditions as it defeats that whole category of mistakes. Fairly innocent but can be so hard to find because of that. A linter would enforce having braces and solve the issue. And for codebases without a linter it is a good habit to just go with the braces.

theshrike79 2y ago

Just having a valid .editorconfig for the project will do a lot for languages without gofmt.

chipdart 2y ago

> This is why I love things like cargo fmt / go fmt / eslint / etc. I agree. Once I had the displeasure of working with a junior dev who was very prolific in posting comments on style and if a space should be at the left or at the right of a symbol. It took me a few days of dealing with that noise to onboard a linter. Even so the junior dev felt entitled to manifest how high their standards were by posting a torrent of comments in the PR that onboarded the team's official linter config. But once the PR was approved and merged, surprise surprise: the junior dev's PR comment metrics dropped from dozens per PR to zero. The style guide didn't even had to be enforced. The last I've heard about the junior dev was throwing a tantrum when one of their PRs received a comment from another team member asking to run the linter because it failed to adhere to the team's official style guide. Apparently the high standards and this attention to detail only went one way.

plasticchris 2y ago
That’s why you hook the linter up so it runs on every commit / push. No need to ask, it always runs. And no need to quibble over style. Don’t like it? Change the linter.
- Aeolun 2y ago
  I very much dislike any process modifying my commit after I submit it. Rebasing becomes hell.
  - plasticchris 2y ago
    run it in a hook that runs before the commit is made
    - cempaka 2y ago
      Yeah we just have any linter failures fail the CI build, but it's left up to submitters to decide how to resolve the violations.
      - 2y ago