Don’t teach during code reviews • Hacker News Board

Re: Don’t teach during code reviews

#71

Post by wk_end » Sun, Feb 05, 2023, 9:45 PM UTC

thinking4real wrote: post #47

password11 wrote: post #17

Earlier quoted context omitted.

I think it's more of the author having a misguided opinion of what teaching is. I've had a lot of good teachers in my upbringing but not one of them has ever thought that a vague/condescending question like " I’m not sure if I understand the whole idea but could you explain what this method does? " would be an effective teaching method. Don't make teachers the punching bag for bad programmer code review. Programmers…

Because to become teachers, you typically don’t have to have your ego brutalized jumping through irreverent hoops. I mean going through engineering school and rigorous STEM degrees I can say that stuff is baked into the formula. You’re derided and dogged and gaslit from the onset. Is it surprising these people graduate, become senior and perpetuate the mental unhealth?

> Because to become teachers, you typically don’t have to have your ego brutalized jumping through irreverent hoops.

No, instead you have your ego brutalized by spending half your youth (not to mention tens or even hundreds of thousands of dollars) getting an undergraduate and master's degree and teaching certification...only to receive poverty wages, pay for your own supplies, be abused by students and parents and administrators and HN commenters alike...

Re: Don’t teach during code reviews

#72

Post by NikolaNovak » Sun, Feb 05, 2023, 9:46 PM UTC

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…

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 guess what I'm expected to say". Typically after a few frustrating minutes I just ask for straight feedback - I am happy and excited to receive constructive feedback and lecturing.

My boss and I had an open discussion and it may be they just need more practice - coaching is a skill like any other and just because you took a two day class doesn't mean you're an expert yet. So we accepted that while he's more experienced than I am and my mentor in many things, he's junior in "coaching" so we sure working on it together :-)

Re: Don’t teach during code reviews

#73

Post by aflag » Sun, Feb 05, 2023, 9:46 PM UTC

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.

If you're looking at someone's job title before making a review I'd say you're doing it wrong. Just point out things that are wrong (actual bugs) and things you think could've be done better. If there are no bugs it's probably ok to even approve it already, unless things were really poorly written. It shouldn't matter whether the person submitting the PR is a junior or the highest ranking developer in the company. Code is code.

Re: Don’t teach during code reviews

#74

Post by bt4u » Sun, Feb 05, 2023, 9:48 PM UTC

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…

You have to fight to produce quality and you have to do it continuously. You cannot just merge in "working(tm) but not using best practices" and then turn on the quality-switch months or years later.

Your attitude is creating a culture where nobody cares and it's why many good engineers end up hating their job.

Re: Don’t teach during code reviews

#75

Post by twawaaay » Sun, Feb 05, 2023, 9:49 PM UTC

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 my experience the law of sunken cost comes into action and people will try to push it as much as possible unless it really has a critical flaw.

For this and other reasons I prefer pair programming as an alternative to code reviews. Work progresses faster when two people do it (vs slower when one has to finish it and then another review it). You actually have two people understanding what happened. And you can avoid all of the drama because problems get fixed as the solution is being designed and written.

And yes, this is a good (or at least better) time to teach. Though I try to not be preachy and instead prefer to demonstrate how I solve problems and comment on why.

Re: Don’t teach during code reviews

#76

Post by onlypositive » Sun, Feb 05, 2023, 9:50 PM UTC

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…

+1 I gave up on the "teaching" years ago, it never did anyone any good and wasted a lot of time.

Now I say explicitly what I think is wrong and what needs changing.

This too can come off badly so I limit reviews to one or two remarks. I also build a relationship with these engineers and explicitly explain how I do reviews different nothing that while I give you solid direction, whether you take my advice is entirely up to you to evaluate and that it is 100% ok with me if you explain why I'm wrong and you're not changing it. - the purpose of my reviews is to catch problems you haven't thought of, if you've already thought about it and made a trade off that's your call to make. (And I trust you to make it)

Translation: I'm too old for this tit for tat shit and have bigger hills to die on than where you put your whitespace.

Re: Don’t teach during code reviews

#77

Post by de_keyboard » Sun, Feb 05, 2023, 9:52 PM UTC

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.

Re: Don’t teach during code reviews

#78

Post by [deleted] » Sun, Feb 05, 2023, 9:52 PM UTC

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.

[deleted]

Re: Don’t teach during code reviews

#79

Post by hgsgm » Sun, Feb 05, 2023, 9:52 PM UTC

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.

"Socratic" method (which was used in a book, not in actual teaching, IIRC), works when the student wants to figure something out on their own but with guidance, like "You Could Have Invented X" blog posts, or tutoring math. It doesn't work when the "teacher" forces it upon an unwitting "student" to trick the student into discovering they are wrong.

Re: Don’t teach during code reviews

#80

Post by [deleted] » Sun, Feb 05, 2023, 9:52 PM UTC

azov wrote: post #16

I’m not sure if I understand what this article has to do with teaching?.. Oh, I get it now, sorry for being slow! Do you think we could rename it to "Don't be an asshole and lie about (not) understanding things" or something along those lines? :) PS. But, titles aside, do we actually want to do teaching during code reviews? There are many activities when teaching and doing are better kept separate (like, you don't wa…

[deleted]