Sooner or later I’m going to end up with an intern, and on that day I’m going to read this again.
I also appreciated the recursion in the article: rather than just state the problem, she addresses solutions. Just like she says to do:
I use criticism to mean feedback that finds something wrong with someone, especially without giving them indications about how they can fix their behaviour.
Instead of saying things like “don’t open such large pull requests” tell someone they should “open smaller pull requests.”
Is that really any different?
there’s a lot of cognitive research that says yes.
Yeah, to add a little more, the research says that when you give a negative direction, the person doesn’t immediately know what action to replace it with, and there’s also a chance they don’t hear the negation in the sentence.
You can see this when you tell a dog not to do something and they start randomly picking other actions to perform instead (including coming back to the one you told them not to, eventually!). As a human example, it’s my understanding that cops are told not use negations in orders they give civilians and each other for those reasons.
Yes! It certainly is. Yelling “don’t run!” at my three-year-old son is unlikely to make him stop. But yelling “walk” might. Focus on the action you want implemented.
You’d think people would interpret it that way, but not everybody does (and not necessarily because they’re idiots).
You’ll get somebody who instead emails you “before I submit this giant pull request, perhaps you’d like to look it over. (Bajillion lines of diff to follow.)”
Yes! The first expresses disagreement with the persons actual behavior, while the second is an instruction towards desired behavior.
“Don’t open such large pull requests” makes is unclear what the fix is. (It could also be not to create any any pull requests anymore)
[Comment removed by author]
“Well, this is a great concept, but it might be better if…” is not what the article means by "positive feedback”. if it’s not a great concept, you don’t need to say so, but what you should not say is just “this is a bad design” with no suggestions as to how to improve it. the examples you quoted later are actually on the right track - they say what’s wrong and offer concrete ideas for how to fix it.
I’m not sure I agree with you here. People are not machines, and feeding them a laundry list of errors is not, in my experience, the way to build a good working relationship and communicate that they are valued and contributing successfully. I’ve found this to be a much more delicate social interaction with more junior people, or people who otherwise have less perceived power than I do in a given setting; with someone you trust, and have history with, it’s definitely much easier to get straight to the “wtfs per minute”.
Sure, newcomers or less experienced people will, by definition, almost certainly not be contributing successfully. The goal of providing feedback is to help them to improve, so you have to know your audience well enough to understand how much critical input they can handle before they shut down and you get the “eyes glazed over, this person seems miserable” response to additional input, or the open-source equivalent, “ambitious volunteer never comes back to the project after the first code review.”
To address your bicycle example: somebody who is inexperienced or new to a project should never be put in the position to independently make big changes in the first place. If I ever find myself reviewing a huge chunk of crazy-looking code that needs to be rewritten, I think it reflects poorly on me: I let this person take on a task that was too large, and/or didn’t keep tabs on them to realize sooner that they were getting lost.
In that kind of situation, I think the best approach is generally something along the lines of, “Thanks so much for taking this on! Nice job with (some aspect of the code that actually looks good, if any), that looks great. I do have some concerns about how the code is structured, let’s find some time this week to take another pass at it together.” If pairing isn’t possible, then you have to break down the big task into manageable subtasks for this person, so that they can succeed at little things. This is what technical leadership is (IMO) all about: helping people and projects and companies grow, one step at a time.
TL;DR: giving technical feedback is good, but it’s better and more important to ensure everybody feels positive and excited about programming by the end of the day. If you don’t get this right, you’re going to have a lot of attrition, whether it’s an open source project or a company.
Okay, but consider a change submitted by an intern or younger programmer, that is 70% good but needs fixes. If one part is implemented in, for example, an especially clear and concise way, say so. That way they keep doing it. You need to axe the bad code of course. That doesn’t stop you from encouraging good code by providing positive feedback.
When I review code, I just write down all my thoughts and impressions as I go. Before I submit I curate my comments to the ones that are important, positive and negative. In all code review software I’ve used, reviews comments are drafts by default, and I take advantage of that.
“Proof by analogy is fraud.”
- Bjarne Stroustrup.
IMO “Critical” feedback is necessary for critical issues, eg) security, deployment. architectural decisions and tabs versus spaces.
I detest painting Engineers as “critical” people devoid of positive feedback.
I wish people choose analogies which are structural in nature rather than just trying to create sense by putting disparage concepts together. Comparing sports to engineering is ridiculous.
In the later there is actually a risk of true danger affecting multiple people.
A sports person is at worst a danger to himself.
A sports person is at worst a danger to himself.
They can be a danger to others in team sports and in sports where you compete against other in close proximity.
IMO “Critical” feedback is necessary for critical issues, e.g.) security, deployment. architectural decisions and tabs versus spaces.
This was discussed in the article. The author agrees with you.
Although for tabs vs. spaces, you can follow the article’s recommendation and say “Use spaces” instead of “Don’t use tabs” and not lose much value, I think.
Did you mean disparate here?
Sports and Engineering are unequal. That’s what I meant.
For example, Engineering requires arithmetic. Engineering requires lots of boring check lists. Engineering has tons of interaction effects, which require extra care. The risk levels are varied. There is lots of detective work. Engineering has a history of disasters which one must learn from. Engineering has more feedback cycles.
This might make Engineers a bit more cynical in asking critical questions. But the positive side exists.
If you really want to see that gleeful positive side, just ask him / her – “How does it all work ?”.
Edit: The crux of the article is that criticism, the negative kind, is not pedagogy. What I mean to imply is that “Dont’s” maybe more in number in Engineering due to the nature of the profession.