Code reviews help me be more confident in my own work. Even if 90% of the time it’s fine, there’s always that 10% where a second pair of eyes catches a mistake or offers a suggestion that makes the code even better.
Code reviews help me be more confident in my own work. Even if 90% of the time it’s fine, there’s always that 10% where a second pair of eyes catches a mistake or offers a suggestion that makes the code even better.
Code review as mentoring can be great, if it's a directed 1:1 effort. Usually, it is not.
This isn't a problem with code reviews, this is a problem with your team's processes that code review has highlighted. These problems will show up in others ways if not during code reviews since clearly there's strong disagreement on coding styles and conventions. So go and fix or implement the style guides, linters and so on.
This is like saying that the way to fix fevers is to get rid of thermometers.
How do you make this converge across dev without endless debate or resentment ?
> linters and so on.
What happens when a rule has to change ? update the entire codebase impacting everyone with conflicts ? tolerate divergence existing code ?
Talking about thermometers, are you actually tracking fevers, or merely minor bad breath ?
How I did it as part of a ~35 person dev team was adopt one of the popular ones from the community (e.g. Google's) - treat it as a benevolent dictator. Apply it to the entire code base in one giant shot (disrupting any pull requests that were in flight but fixing them proactively), then enforce it with tooling. Leave the door open to anyone who disagrees with the defaults. It's on them to propose the change with rationale then put it to a quick majority vote with a subset of the team. I think 2 things have actually changed since the very beginning (line length from 80 -> 120 and something else that I don't remember right now).
When something changes, making a sweeping change is part of accepting it - if it's too risky/disruptive then it has to be very valuable to do.
The same people who are perfectly happy to do a back and forth, adding 3 days of code review time, would never go in and refractor existing code to "fix" stylistic issues. It's a cognitive bias that we think everything is up for philosophical debate in reviews.
Btw, I'm one of those people who drove stylistic debates in an organized fashion. I learnt that you simply can't police everything, even if it we pretend that it's a good idea to do so. It's much better to share ideas and have them spread through voluntary mechanisms, such as dedicated tech talks, mentoring, postmortems and other deliberate knowledge sharing.
That said, automated enforcement of pure formatting can work well if there's standard tooling, e.g. gofmt and rustfmt. But contrary to reviews, that's a way to suppress debate during day to day business.
Btw, my experiences are from a mega corp, not claiming it's true for every org.
plus, software engineering is a team sport, so if something works but is opaque, that's a fine thing to point out in a review, otherwise one runs the risk of being The "Foo Component" Owner™ and that's usually no fun and is for sure not healthy for any reasonably sized org
I am also a monster fan of the "Apply Suggestion" button in GitLab, which allows me to meet the reviewer half-way by even doing the legwork for the suggested change. If they agree, push button and we're back on track. It's suboptimal for any process that requires/encourages signed commits, but damn handy otherwise
Testing is pretty awesome process to find bugs.
And never learned how to properly test software... Or earn 10x as much and consider it not a priority.
In my experience, it's hard to have a dev as a good tester. I'm ok at it but not as good as a pro QA person. Good devs look for shortcuts and efficiencies, which is the opposite mentality for good testing.