41–50 of 304 posts
It all depends on who the parties are, their relationship to one another, their relative knowledge of the tech stack, application and problem domain, and the nature of the code being reviewed itself.
Your reviewing style should ideally be tailored to the above contexts. Conversely, if you use the same exact process everywhere, you’re probably doing it wrong.
> “I had a hard time grasping what the method does. What about changing the method name to openRequest() to make the methods objective clearer and improve code readability?”
That suggestion requires grasping what the method does.
In the original article the feedback is given after the submitter had explained.
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…
Yes.
Expert intuition is a tacit skill one can only learn on the job. By getting feedback from an expert in real-time. It cannot be done as a separate exercise. There have to be real stakes and the work has to be real.
You wouldn’t expect a surgeon to not get feedback during their first surgery would you? But you also wouldn’t want them to cut somebody open just for learning in the absence of a medical need.
We even call this “supervised learning” when a computer is doing it.
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.
For example: underlying does a task that is correct and valid but not what the higher up envisioned or hoped for. In this case you’re just going to come off as a prick for “gently guiding me” to a conclusion I disagree with, but which I’m not allowed to (non confrontationally) articulate as you’ve corralled this conversation down one narrow path based on your personal preference
As someone who while I was a junior was starved for someone to actually teach and mentor me, I’d say this scenario played out much more often than one where I was given valuable education. yrmv
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.
Isn’t that more “don’t give vague feedback” or even “don’t misdirect the reviewee”? “I’m not sure if I understand the whole idea but could you explain what this method does?” misdirects by suggesting the reviewer thinks they need education, rather than that the reviewer thinks the code can be clearer.
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…
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?
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…
This is quite common in teaching, and assessment, to the point that some people think it's synonymous with teaching. It really isn't a good way of teaching, and I gather that modern teacher training teaches you not to do it. https://betsysneller.github.io/pdfs/Labov1966-Rabbit.pdf
> He tells me that the teachers had already decided that many of the school children didn't have any language at all: they didn't know English and they didn't know Chamorro. When he asked them how they knew that, they described the very same kind of testing procedure that I have observed and reported in mainland schools. […]
> The children's response to this test, in general, was to say as little as possible. […] James is one of the most talkative children in the group. Others said much less. Some were paralyzed into silence by the request for display: […] To all these questions, Eunice presented a stubborn resistance. Finally, she produced a minimal response to the teacher's verbal bludgeoning: […] The teacher-tester is a pleasant person when you meet her face-to-face as an adult. […]
> A third characteristic of adults' talk to children is deliberate and obvious lying. The teacher-testers frequently try to force answers to known-answer questions by claiming that they don't know things which they plainly do. As the children follow the strategy of saying as little as possible to stay out of trouble, they frequently answer with "Uh-huh" or a shake of the head. The teacher could simply point out that the tape recorder wouldn't pick that up. But instead she says, "I don't know what uh-huh means."
---
In fact, the author's preferred code review:
> “I had a hard time grasping what the method does. What about changing the method name to openRequest() to make the methods objective clearer and improve code readability?”
is a much better lesson than the "teaching attempt" it replaces. I would call teaching the primary purpose of code review, with the resulting codebase improvements a useful (but necessary) side-effect. The alternative, of just silently fixing the code, is worse because it doesn't stop the same mistakes being made again.
The worst part about this is that it forces more unnecessary communication in the code-review process...
If you're reviewing, then say what you think needs to change and why...
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.
I think the real problem here is that because everything is going online-only, we're falling into the trap of making everything public and recorded forever . Maybe I'm just too damn old, but I don't like this trend. When code reviews were in person, as a senior person, I could deliver feedback without that feedback becoming a public albatross stuck around someone's neck. We all screw up and miss things. Sometimes we…
Emails that have a long list of CCs suddenly become a political hothouse of "If I say X in front of Y it will seem like a criticism and they will clam up and then the whole thing becomes something the "positional authority" has to adjudicate not an experience led discussion"
I feel that this problem is the very reason FOSS mailing lists (ie Linux) were so brutal - it's that or you risk lack of clarity.
I know I sound like someone complaining about woke snowflakes, but there is a line somewhere around here and I wish I knew where it was