Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Don’t teach during code reviews

michaelagreiler.com

Post reply on HN
Print view View original

91–100 of 304 posts

Re: Don’t teach during code reviews

#91

Post by dgb23 »

PragmaticPulp wrote: post #64
sidlls wrote: post #24

I use junior engineers’ code review submissions as an opportunity to teach, and any senior engineer who doesn’t is committing professional malpractice. They have to learn, and that requires someone (in the case of a code review) to teach them. One can do it without being condescending, though The Socratic method for teaching is actually quite good, and it can be employed without condescension.

> The Socratic method for teaching is actually quite good, and it can be employed without condescension. Socratic Method is hard to use without coming off as patronizing, even if you think you're being careful. If you're genuinely jumping in to ask real questions to understand the problem, that's great. Most of the time when I see people use Socratic Method in the workplace it's because they think they're doing the o…

I think there's value in having a small set of 'goto questions' or maybe rules that are well known and regularly used. Then you can go over them together in the case of a review, a debugging session or to clear up a problem.

I don't know how that relates to the Socratic method. But I think it's useful, if done honestly. It's like a little ritual that you do together and it serves both sides as a guideline in the moment, but also helps to converge to a common way of communicating things and writing code.

Re: Don’t teach during code reviews

#92

Post by azangru »

sidlls wrote: post #24

I use junior engineers’ code review submissions as an opportunity to teach, and any senior engineer who doesn’t is committing professional malpractice. They have to learn, and that requires someone (in the case of a code review) to teach them. One can do it without being condescending, though The Socratic method for teaching is actually quite good, and it can be employed without condescension.

> any senior engineer who doesn’t is committing professional malpractice

Interesting. Why not turn it around and say that any junior engineer who doesn't use comments left by his more experienced colleagues as a learning opportunity is committing professional malpractice?

Re: Don’t teach during code reviews

#93

Post by EdwardDiego »

throwaway892238 wrote: post #61

The worst thing during code reviews is people nit-picking my code or making comments that aren't important questions or mandatory changes. I didn't post this code review to get your two cents! I need to ship this code! Either give it a thumbs up, ask a relevant/important question with context, or ask me to change something that needs to be changed. Otherwise, shut up. I can handle nit-picks, compliments, curiosities,…

I left my last job because of the org issues that led to it being stupidly hard to land PRs, because of one project lead always demanding significant changes because he didn't like X, do Y, when both were valid approaches, he just preferred Y.

You'd do Y, and then he'd complain about the changes that occurred because of Y, so now do Z. And so on.

I'd throw my PRs up as drafts early on, and invite his feedback as soon as possible to try to avoid this, but nope, it all came when the PR was ready for merging.

In the end I decided to work in an area that this lead didn't like and didn't know about, just to get code merged without spending two weeks in review.

Re: Don’t teach during code reviews

#94

Post by globalise83 »

de_keyboard wrote: post #77
twawaaay wrote: post #75

Code reviews are bad anyway. This is the worst time to try to fix anything. The author has just finished (or thinks he finished) the work and any attempt to change anything by the reviewer is going to elicit resentment in most people. Try to tell the person the change cannot pass and you can make an enemy. Or you let the change in because you like the person. Either way, it is bad. And all this for naught because in…

But then you are working on half as many things.

Often working on half as many things and executing them well is a good idea...

Re: Don’t teach during code reviews

#95

Post by njharman »

The post isn't about not teaching during code reviews. It's about not doing it badly, duh? I'm actually shocked at his example. It's obviously bad practice (and just jerk/toxic behavior (don't fucking be coy, EXPLICIT > implicit)). It seems like strawman or cherry picked. I never experienced in 25 years.

Quotes from article

> It’s not bad to “teach” in code reviews

after example of "proper" review

> The learning in this type of comment

Should absolutely "teach" during code reviews. Developers should always be teaching and learning from each other.

Re: Don’t teach during code reviews

#96

Post by kissgyorgy »

Ensorceled wrote: post #65

As a manager, it drives me crazy when some feature doesn't make it into the sprint because some "senior engineer" decides that this is the time to teach a junior/intermediate the proper way to do something with a long drawn out "teaching process" via PR comments. it's especially galling if the original PR was working, reasonably well written, no major flaws and they forced the junior to rewrite because it wasn't "bes…

People without technical background shouldn't be allowed to be managers of highly technical teams.

Re: Don’t teach during code reviews

#97

Post by PragmaticPulp »

NikolaNovak wrote: post #72
PragmaticPulp wrote: post #64

Earlier quoted context omitted.

> The Socratic method for teaching is actually quite good, and it can be employed without condescension. Socratic Method is hard to use without coming off as patronizing, even if you think you're being careful. If you're genuinely jumping in to ask real questions to understand the problem, that's great. Most of the time when I see people use Socratic Method in the workplace it's because they think they're doing the o…

Yeah ; I'm super open to coaching as opposed to mentoring, but last few years, when my managers tried to employ the coaching / questions methodology, it was obvious and painful : they did not feel like open questions to discover something together ; they felt like fake questions that had a "right" or rather "expected" answer, so it did not feel at all like "me figuring things out for myself " but rather"me trying to…

I had some college professors who used Socratic style in class and I enjoyed it. However, that was literally a teacher/student relationship that I signed up for. Furthermore, the professors were usually genuinely curious about my responses and wanted to explore them when they didn't necessarily agree with the expected answers.

In the workplace, Socratic method just feels like an unnecessary power move. Someone is trying to cement their role as teacher and the other person as student.

I had a manager who liked to use the Socratic method for everything. He communicated everything in the form of questions. If you gave the "wrong" answer, he'd give a sharp sigh and then rephrase the question, giving you another change to give a "better" answer.

This method was mildly annoying when he was right, but it was completely disastrous when he was wrong. He'd often arrive late to a situation that people had been dealing with for months and assume he knew exactly what was going on, better so than the people involved. He'd start his Socratic questioning, but you weren't allowed to explain the context or how you arrived at a solution. Your only option was to navigate his Socratic questioning until you could gingerly explain that he was missing a key constraint, or that we had already tried that, or that he had received bad information, and so on. It became a tool for him to control the conversation and put you in your place at the same time.

It was a very sad time in my career. I'd arrive to every meeting feeling like I was about to play a psychological game of "guess the right answer" as I navigated the Socratic questions until we could get him to reveal what he was really thinking, or why he was so frustrated, or why he thought we were wrong, or any other number of issues that he just wouldn't tell us.

Re: Don’t teach during code reviews

#98

Post by [deleted] »

silveroriole wrote: post #35
HL33tibCe7 wrote: post #7

The proposed approach to code review in the mentioned medium article makes my skin crawl. So patronising and passive-aggressive.

There’s no link to the original medium article but I have a feeling the author was female or some other minority in the industry. I might be totally off base with that though. I agree that the wording is very cringy, but if I may offer one defence: I also learned to do this sometimes because otherwise juniors wouldn’t listen to me at all. You had to make them think it was their own idea or they were magnanimously gra…

[deleted]

Re: Don’t teach during code reviews

#99

Post by Ensorceled »

kissgyorgy wrote: post #96
Ensorceled wrote: post #65

As a manager, it drives me crazy when some feature doesn't make it into the sprint because some "senior engineer" decides that this is the time to teach a junior/intermediate the proper way to do something with a long drawn out "teaching process" via PR comments. it's especially galling if the original PR was working, reasonably well written, no major flaws and they forced the junior to rewrite because it wasn't "bes…

People without technical background shouldn't be allowed to be managers of highly technical teams.

I agree. Wait, do you think I'm non-technical?

Re: Don’t teach during code reviews

#100

Post by rmk »

I've had instances of junior people who are simply not interested in learning and suffer from a massively inflated sense of ability and seniority, in practically every job. When reviewing code submitted by such people, "don't teach during code reviews" is actually good advice to the senior person. The senior person is saved the angst and futility of the effort. As a consequence, the other benefit of the code review, which is to catch bugs before they escape to the field, is lost as well.

For new people or junior people who have the right attitude, this is poor advice. Where else will people get habituated to the standards, preferred idioms and implementation quirks of the team/company/product/subsystem?

Post reply on HN