4 ms·

Like everything in software development, it comes with "it depends". - getting peer review on iffy code = 100% - waiting for a PR to be approved for 6 hours,

by keyle 4y ago

Like everything in software development, it comes with "it depends". - getting peer review on iffy code = 100% - waiting for a PR to be approved for 6 hours, by 2 parties when it's typo fixes and adding some comments = ... As a senior dev with over 20 years, I value code reviews from even junior devs. Heck it might spark a conversation that might mean everyone learn something, or get a refresher. It's also simply put: 2 brains are better than 1, particularly with codebase fatigue. And by the way, in dynamic languages: even more so. Dynamic languages save you time in the writing side of things, which you pay back in the testing. Make no mistake, you are the compiler. So my philosophy is if you feel this might benefit from another set of eyes, seek reviews. If you feel you're going to waste someone's time, don't. But mission critical code, e.g. handling payment information, user privacy impact of any kind, business logic, should go through code reviews, on top of QA and UAT. Trust people, don't trust code. And if you can't trust your people, you need new people(!)

eitland 4y ago
Agree very much on this. I'd like to expand a little bit on this part: > I value code reviews from even junior devs. In fact it can be really valuable. If they don't understand it it means it is either too complicated or it might mean it is time to explain the necessary concepts to them.
- keyle 4y ago
  Yes and also for yourself, if you can't explain it to an intern, are you really understanding your code?
  - JohnFen 4y ago
    I've found that the biggest benefit of code reviews is the part where I explain my changes and why I made them as I did to the reviewer. When a review has found problems with my work, 90% of the time I've spotted them myself during that process.
atq2119 4y ago
> - waiting for a PR to be approved for 6 hours, by 2 parties when it's typo fixes and adding some comments = ... This is arguably not a problem with the review process, but a problem of not being able to maintain an asynchronous workflow. Git itself is made from the ground up to allow you to make a change, send it out, and then go focus on something else for the next few hours. It does require some amount of learning your tools (e.g., cherry-pick and rebase), and unfortunately most of the forges that people build on top of Git are quite bad at this. (Gerrit is the best one I know for this particular problem space; unfortunately, it has some other downsides.)
ofrzeta 4y ago
Great comment. +1 for the concept of "codebase fatigue".

by keyle 4y ago

eitland 4y ago

Agree very much on this. I'd like to expand a little bit on this part: > I value code reviews from even junior devs. In fact it can be really valuable. If they don't understand it it means it is either too complicated or it might mean it is time to explain the necessary concepts to them.

keyle 4y ago
Yes and also for yourself, if you can't explain it to an intern, are you really understanding your code?
- JohnFen 4y ago
  I've found that the biggest benefit of code reviews is the part where I explain my changes and why I made them as I did to the reviewer. When a review has found problems with my work, 90% of the time I've spotted them myself during that process.

atq2119 4y ago

> - waiting for a PR to be approved for 6 hours, by 2 parties when it's typo fixes and adding some comments = ... This is arguably not a problem with the review process, but a problem of not being able to maintain an asynchronous workflow. Git itself is made from the ground up to allow you to make a change, send it out, and then go focus on something else for the next few hours. It does require some amount of learning your tools (e.g., cherry-pick and rebase), and unfortunately most of the forges that people build on top of Git are quite bad at this. (Gerrit is the best one I know for this particular problem space; unfortunately, it has some other downsides.)

ofrzeta 4y ago

Great comment. +1 for the concept of "codebase fatigue".