"Work relationships might only need a ratio of 1:1, but especially as an engineer, where the default's to critique and only pick out the bad parts in code reviews, these really help!"
I'm on mobile so I can't pull a source, but research for work relationships generally agrees on a ratio of around 6:1 positive to negative ratio for negative feedback to be taken in a positive way.
You're correct that engineering culture falls FAR short of this, and leads to some really bad outcomes as a result.
I'm not exactly a software engineer, but I have a really hard time imagining how 6 in 1 statements about anything could be complements. In my area, most work is average (by definition) and the main reason you show your work to people sitting in the chair next to you is so that they can catch your trivial mistakes. (Kind of like code review.) I do know that in the average program there are far more bugs than clever inventions.
That's 100% true, but being a good software engineer isn't about clever inventions. It's about reliably doing the right thing, and that in and of itself is praise-worthy.
When someone writes tests I will always call those tests out. Because I know that writing tests isn't always the funnest work and as such I'm really grateful that they took the time to do it!
Or what about when someone writes a really good code review? They took the time to review your work and that's another time to say "hey thanks for doing that!".
If you really look for it there are dozens of things a day that your teammates do for you that are real chances just to say "hey it's really cool that you did that". When you really start to look you'll find that you can have a really positive impact on your team by calling out all of the good things you see going on around you. It really does make a huge difference.
> When someone writes tests I will always call those tests out. Because I know that writing tests isn't always the funnest work and as such I'm really grateful that they took the time to do it!
This sentiment, of applauding people for good work , works both ways. It makes the applaud-ee feel good, it does believe me, even if its simply because someone noticed it and cared enough to say it. It also makes the applaud-ee tad bit more respectful towards the applaud-er and try to return the favor. Add more people in this and its a recipe for a healthier work environment.
Also, it is how you build a culture around behavior. For instance, if you’re constantly applauding tests, then that makes a norm, and reinforces the behavior more than a curt “write tests”.
In between those trivial mistakes is a lot of functional code I didn't have to write myself. Very often, that's code that I personally no longer have to write (if I'm reviewing it, there's a good chance it would've ended up on my place if they didn't take care of it for me), or that I get to leverage in my own code. That's stuff I can thank them for.
Phrasing is important. "Here's 3 things you fucked up, fix it" is negative. "Here's 3 minor nitpicks. Otherwise, LGTM, thanks for tackling this! :thumsup:" is overall positive. Both could be describing the same code review. "While you're in there, could you add unit tests for X and Y?" is asking for a favor you can then thank them for doing, "X is missing unit tests, why?" is demanding answers for a mistake.
You can still help set the tone on the receiving end of code reviews as well. "Good catch!" when people spot significant bugs in my code, reacting with thumbs up emojis when people voice their concerns. Elicit feedback, suggestions for improvements - instead of "what is everything this code does wrong", it's "okay, here's a first draft - how can we make it better?".
The goal is not to deliver perfect code. That's impossible - we're human. Trying to do so anyways is counterproductive - static analysis, and a second set of fresh set of eyes will find some bugs faster. Seperating onself from one's work like that can be difficult - but between setting an example yourself, and showing geniue appreciation for the person's work even as you're finding issues in the code can help.
Mostly agree. Might want to rethink the nitpick thing.
> The goal is not to deliver perfect code. That's impossible - we're human. Trying to do so anyways is counterproductive.
If it's not important why are you saying anything?
If it's code style, well if it's code style I'm going to ignore you. And if pressed I'm going to tell you to fix the linters because I ain't got time for this shit.
> If it's not important why are you saying anything?
If I can take the 5 seconds to type up something I've seen, you can take the 5 seconds to fix it. We're not trying to ship perfect code, but we are trying to ship good code - so we might as well take advantage of any easy wins like fixing spotted issues.
Sometimes my "nitpicks" turn out to fix/catch serious bugs that I didn't realize were bugs. Sometimes they're unimportant and ignored. I don't always know which way it's turning out ahead of time.
Sometimes they're important, ignored, and then things break when it's checked in. Whoops. Easily fixed though.
> If it's code style, well if it's code style I'm going to ignore you. And if pressed I'm going to tell you to fix the linters because I ain't got time for this shit.
If it's exact whitespace rules, sure, I probably don't care either. I might mention it in passing so you can fix your autoformatter rules so the people who do care get off your back.
If you're making me spend my time adding CI steps/rules, and build server time executing them, because you can't be bothered to match basic function casing rules? I'm going to be annoyed. Especially for more important code style issues that are hard to teach the linter about, like "dnt abbrv evrythg its unrdble", and "don't name your indicies ijIl in the same scope."
Code formatting, code style, and linting is a solved problem. If you're relying on individual developers to setup their Dev Env to do this you're doing it wrong.
You need to have it automated with commit hooks. Anything else is a solution that results in aggregating nitpick reviews that ignore glaring structural problems.
> Code formatting, code style, and linting is a solved problem.
Only the low hanging fruit. Yes, they'll reduce tabs vs spaces debates. No, I haven't found a linter that can suggest good function naming, and if I did it probably wouldn't work across all of the 10+ languages I get to use at a typical gamedev job. Most stylers and linters don't even catch basic spelling mistakes.
Sure, use tools to help, and push to adjust your coding standards to conform to the limits of those tools where practical and sensible - but even that's no panacea, it won't solve everything.
> If you're relying on individual developers to setup their Dev Env to do this you're doing it wrong. You need to have it automated with commit hooks.
Considering you commit locally in git I'm confused how you're supposed to avoid having any dev env setup. Maybe you'd suggest imaging your dev machines, but that's a massive waste of time for smaller shops where your install requirements change more frequently than your image gets installed, and tend to be user specific. You want a living document/checklist that you can update before setting up a new box, and default settings files, not constantly rebroken over-automation. Nor is the average FOSS contributor going to download gigs of images just to format a bugfix patch when trying to contribute back upstream.
> Anything else is a solution that results in aggregating nitpick reviews that ignore glaring structural problems.
You can't technology your way out of a social problem. I have seen low-automation environments without this problem, and I have seen high-automation environments with this problem.
You're not going to convince your coworkers to spend more time/brainpower into spotting glaring structural problems by ignoring half their feedback. They'll rightfully assume you'll likely ignore half their suggestions for structural fixes too, wasting their effort. Especially if your claim is that you "ain't got time for that" - don't got time for a quick and easy code style fix? How the heck am I supposed to convince you to spend the time to do more serious refactoring to fix structural problems, or add unit test coverage to catch what my reviews will miss, then?
I'm not saying you should manually fix up every style issue in your code, but if code formatting/style/linting is truly such a solved problem, then it's going to be way more constructive to setup the configs/rules/hooks to fix it for everyone once and for all, than to simply ignore it.
Because nothing ever ships without issues. When talking about end products, we're often - if not usually - talking straight up known bugs, not mere nitpicks. Hell, I'm willing to OK known bugs in changelists under some circumstances - like being less buggy than what's currently checked in!
Serious bugs can be a good reason to hold off on something. But nitpicks - say, a minor quibble about function naming is a subjective personal preference that should never devolve into a lengthy debate holding up an important improvement that otherwise checkin. We both have bigger, more important dragons we could use our time to slay. It's worth discussing briefly - maybe we can achieve consensus and find a better option - but if you want to check in your code as is, right now, for pretty much any reason whatsoever? Go for it. We can always make a followup changelist. Hell, please do - those are easier to diff! Even if we're working in "commit straight to shared master branch in perforce" style. If you're using git, but are worried other people about to start refactoring and want to merge into the main work branch ASAP? Go for it. Naming debates can wait. Typos can wait. There's always the next changelist. Just have a general dread of having lots of files checked out? Go for it. Make your commit. The issues are nitpicks, not blockers.
I've seen plenty of good work - changelists that are objectively improvements - die in analysis paralysis and never get checked in because of quibbles over less important details. It sucks. Minor naming quibbles should not be a cause of this. Significant design problems - maybe - but not minor easily fixed after the fact naming quibbles.
"LGTM otherwise!" is also a good finisher to signal "Okay, I've gone through everything, I think I won't find anything else." In more relaxed review environments, it's a signal that I don't feel the need to re-review your change - maybe with minor fixes for outright bugs - before you commit. It is carte blanche to move forwards.
You can say "good job!" in response to mediocre work. You can say thanks for simple interactions that are just people acting as expected. You can build a culture where trust is both expected and repeatedly acknowledged.
I think there's a lot to be found in some of the business practices we've cast off, like firm handshakes and business cards - they matter as simple acknowledgements. Literally, just serving the same purpose as a TCP ack. "I'm still receiving you, go on".
Distributed systems rely on a lot of tradeoffs about how often you reconcile state and how much you expect the other parties to have successfully executed on what they say they have. Human relationships are a vastly more complicated distributed system than we've otherwise imagined.
So now when I do code reviews, I also verbalize all the things I like and agree with, like sharing my inner dialog, just like subjects are asked to do during usability tests. This helps me convey my respect and appreciation for my coworker(s) and their effort(s). I especially call out when I learned something from them.
Further, we recently got a puppy. I'm struggling to practice positive reinforcement training. The trainers make it look easy. But when I finally connect with my dog, successfully training a new behavior, it's like magic. At first, it feels (and looks) weird constantly praising, but after a while it becomes the new normal.
I just wish I knew all this stuff much earlier in life.
I read this with great interest before noticing the retraction. Has there been any other research like this without the caveat?
> *Authors’ Note: The journal that published this study has since expressed concern about the data. We first became aware of this research in Kim Cameron’s book, Positive Leadership. Like many others, we were distressed to learn of the incorrect data in the Heaphy and Losada research and we immediately ceased our citations upon learning that the study wasn’t correct. But we do believe the basic assumption and premise that leaders should provide more positive than negative feedback is correct.
I'm on mobile so I can't pull a source, but research for work relationships generally agrees on a ratio of around 6:1 positive to negative ratio for negative feedback to be taken in a positive way.
You're correct that engineering culture falls FAR short of this, and leads to some really bad outcomes as a result.