News for hackers. Stuff that matters.
Re:Don’t Teach During Code Reviews by Ensorceled on Sunday February 05, 2023 @04:55PM (#34669950)
Sure, we have code reviews for a reason, to make a better product.

There are better places, and ways, to teach junior developers than long, painful code review processes.

Re:Don’t Teach During Code Reviews by lelandfe on Sunday February 05, 2023 @05:25PM (#34670323)
Code review is the most common arena for getting feedback on my code. My team doesn't pair very often, though I think that's more effective. I don't believe I have other opportunities for it...?
Re:Don’t Teach During Code Reviews by astrange on Sunday February 05, 2023 @06:25PM (#34670961)
If you're getting complete rearchitecture feedback in a code review, something's wrong because that's too late for it. Especially if it means rewriting later changes they haven't put up yet.
Re:Don’t Teach During Code Reviews by dilyevsky on Monday February 06, 2023 @02:25PM (#34682753)
The wrong part happened when sender went to work on something alone for a long time then sent a giant PR all at ones instead of progressive set of changes. Don’t blame the reviewer for having to either compromise their integrity by letting it slide into the codebase or getting onto the likes of TP’s shitlist by putting their foot down.
Re:Don’t Teach During Code Reviews by penjelly on Sunday February 05, 2023 @05:43PM (#34670531)
i disagree, it is the place. The work shouldnt be at risk of missing release to begin with, timelines should not assume a senior dev works the issue.
Re:Don’t Teach During Code Reviews by fatjokes on Sunday February 05, 2023 @05:47PM (#34670568)
Like what? What's that Confucius saying: "I hear and I forget, I see and I remember, I do and I understand"? Nothing beats teaching using a real example that the parties are hands on with.

I get what you're saying. Delaying a milestone to have a long convo in a CR is not good prioritization. But as the technical manager it's on you to budget more time for junior devs to land their changes.

Re:Don’t Teach During Code Reviews by sidlls on Sunday February 05, 2023 @05:51PM (#34670614)
I just strongly disagree with this. The context of a code review is the perfect opportunity to teach, much like a technical design review is for system designs, and having them sit as an observer during an incident response is for triage/debugging a live system under stress. There are no better settings for teaching these skills to juniors. And you should really let your senior engineers take advantage of these times to impart their wisdom. It will help them grow junior engineers into high quality seniors that will help make your team’s output better and faster in the long run.
Re:Don’t Teach During Code Reviews by Ensorceled on Sunday February 05, 2023 @06:00PM (#34670719)
> The context of a code review is the perfect opportunity to teach

I think we are just talking about different scales and different senior developers. Every code review is either teaching something, finding missed bugs, find better ways to do things that a senior developer makes full use of.

If not, your code review is just a preformative waste of time.

I'm talking about simple PRs, the kind I would assign to a junior developer, that are actually finished, getting held up while a "teachable moment" unfolds.

Re:Don’t Teach During Code Reviews by sidlls on Monday February 06, 2023 @07:55PM (#34686996)
But "simple PRs" with poor implementations compound over time and make for outsized tech debt in the future. I agree that addressing specific major issues within the PR (e.g. comments) is not a good approach. If there are "enough" (for some relatively subjective definition of that) "teachable" things, it's likely an in-person (or at least chat thread) conversation is warranted. Under normal circumstances (and I'd hope a junior engineer isn't making a PR to patch something for an emergency/incident resolution), a PR should be fine to hold up, as the release timeline should account for the fact that junior engineers will have longer PR cycles.
Re:Don’t Teach During Code Reviews by strix_varius on Sunday February 05, 2023 @05:59PM (#34670702)
> There are better places, and ways, to teach junior developers than long, painful code review processes.

If I had to choose the best single way to grow as a software engineer, it would be through code reviews: both giving and receiving.

That said, "long, painful" code reviews is a red flag for your team. A code review should have one of four outcomes:

1. Lgtm.

2. Approved. Left some suggestions for your consideration.

3. Specific changes requested.

4. This whole approach needs to be re-evaluated, let's talk.

_None_ of those should be long or painful. It sounds like your team might be doing #4 in the PR itself when really that's an exceptional case that should make it to a usually-synchronous discussion.

Re:Don’t Teach During Code Reviews by dilyevsky on Monday February 06, 2023 @02:21PM (#34682656)
> There are better places, and ways, to teach junior developers than long, painful code review processes.

Like what? And dont tell me it’s some kind of lecture/lab-style group meeting for 1-2hrs