3 ms·

IMHO the syntax & other nitpickable matters should be caught by the linter. The regressions should be caught by the tests. And the fact the code runs and works

IMHO the syntax & other nitpickable matters should be caught by the linter. The regressions should be caught by the tests. And the fact the code runs and works should be a given. The discussion should be around the architecture of the solution & the long terms consequences of solving it that way. I also like to do the reviews in person, where we just go over the code together and talk about it. Small remarks can be immediately fixed Also I think work in progress should be pushed often in a wip PR/MR, that way people can give early feedback

luismedel 2y ago
Completely agree. It's a peer review, Not a control layer.
- hiatus 2y ago
  > It's a peer review, Not a control layer. Peer review is absolutely a control layer. Having a review process in place means it requires coordination or at least negligence for a developer to introduce malicious changes into a project. At the very least, peer review can act as a quality gate for production changes.