Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

One goal that I think often gets missed out of code review guides is education.

To me, code review is not just there to produce better software, but also better developers.

It's a great place to talk about tradeoffs, alternative approaches, reasoning behind changes, etc.

Maybe less useful in huge projects with many contractors you're never going to work with again, but I've seen the mentoring aspect have great benefits in small teams.



Couldn’t agree more. I am lucky to have worked with two individuals early in my career who would take the time to really help develop team members by way of thoughtful, detailed PR comments. I attribute much of my own growth due to this, and as such I always try to do the same for others. I care deeply about it and consider it to be one of the most important parts of the job. It saddens me that most people (IME) don’t feel the same.


On the other side of the spectrum - code reviews have never, not even one single time, given me any real opportunity to learn something of value. They have only ever been someone else shoving their preferences onto my coding style (for better or worse)

I was shocked at my first job to realize the code reviewer didn’t even read my code, merely glanced over and demanded style changes (which were not consistent from review to review in the slightest)

It really felt more like how you hand your advisor your thesis with obvious easy to fix errors so he doesn’t compulsively decide “well something must need improvement” and make you fix something difficult


From your multiple comments on threads it seems like your ego is strongly attached to your code - if you truly never, not even one time, derived value from a code review, either you always choose to work in toxic environment/s or you're not willing to accept feedback or defend your "coding style" well enough to align colleagues with your opinion. I desperately miss code review feedback at this point in my career after excellent engineers made me question my strong beliefs and empathise with theirs helped me build my path.


I do a lot of mentoring and I’m not sure that code reviews are really the right place for it. Comments in a code review can be fine for small things like “oh you just rewrote a function that already exists over here- maybe you’d like to use that instead”, but those things don’t really matter much in the end.

Teaching people how to design applications is a lot more useful, but code reviews are really not the time and place. For one thing, making someone go back to the beginning when they have a working (if flawed) PR is going to demotivate people and strip away their confidence. It’s also going to really limit your ability to ship quickly. Even if you put those challenges aside, the review experience for GitHub PRs (which I’d guess is how most people are doing reviews) doesn’t allow you to comment on code that wasn’t part of the diff. This makes it really hard to discuss the broader impact of a change i a PR because you can’t add context inline about things someone missed.

After trying and failing to do good mentoring through code reviews I’ve mostly given up. I do use them as a chance to see where people might benefit from some mentoring, but I keep my reviews quick and lightweight. If I don’t see anything wrong with the PR I approve it. Mentoring happens during dedicated time I put on people’s calendars to focus just on mentoring outside of a PR.


Absolutely - I don't see code reviews as a replacement for mentoring, more as something to augment it, or direct future mentoring sessions.

I've had quite a few experiences where I've suggested a change in a code review, and found out that the author lacked knowledge in that area. That's then turned into a future topic to focus on in future mentoring sessions.

Agree about design too, I don't think code review is really the place for that, since it's often a bit late by then (plus it's hard to get a sense of the overall intent and architecture from code chagnes alone).

It's a shame it doesn't happen formally more often, but one place I worked in the past always had a separate design review (which needed at least one person to approve) before coding could even start. This was fairly informal, but caught a few major potential design problems early on.


Your point about the inherent conflict between educating juniors & having lots of contractors is an important one. The most messed up teams I worked for were 1/2 (junior) contractors and this was a big issue. You basically have a team of bad developers you don't really mentor & expect to get better. As to the why, you can talk to management. The senior engineers lost that fight.


Junior contractor is a position that simply shouldn't exist.


Yup.

There's an entire industry of body shops that will send you lightly trained 22 year olds, in size.


Agreed. And, sadly, besides providing zero education, some code reviews even turn into spaces of abuse of political power when conducted by unprepared reviewers.

I've been unfortunate to work with one of those, some years ago. I decided to pretend I enjoyed the style as soon as I realized she suffered from a fragile sense of authority. In hindsight, that doesn't seem to have been a good decision.

It was clear she felt affronted by any deviations from the intended standard. But the feedback mostly comprised extremely vague, subjective, non-actionable ad hoc prescriptions. I carefully read through my reviews and the ones received by my colleagues, but was unable to devise any pattern. Ultimately I decided to simply code as fast as possible so that I could collect the feedback earlier and get rid of the damn thing.

That way I could kind of cope, but after some months I was really burned out and struggling to sustain any interest about my tasks.


Additionally, the Education is both ways - the reviewer is also getting educated along the way, especially from new developers who have a more fresh knowledge of things.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: