1. [Comment removed by author]

    1. 11

      “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.

      1. 3

        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”.

        1. [Comment removed by author]

          1. 1

            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.

        2. 1

          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.