5 ms·
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,
by 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 agoYou've never been on a team with two people with opposing opinions I guess.
- Quekid5 6y agoJust 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 agoStyle 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 agoI 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 agoSmart tabs is halfway there.
- brigandish 6y agoThis would be my number one wish for tooling.
- goto11 6y agoWhy 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 agoAfter 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 agoEvery have more than one reviewer in your CR?
- mrighele 6y agoHaving 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)