[watwut]

Cause reasons for fear are not something people would tell you. I was in similar state twice and neither time I would be eager explain that to someone external.

I was "afraid" to commit in a team where code review evolved into huge micro-management with inconsistent requirements on what good code looks like. The reviewer iteratively forced you to change it again and again each time with "this is bad code cant go in" comments. But you could not learn what he considers good code, cause it was different every time. I left after.

Second time I was "afraid" to commit to part of code when our main architect had completely disproportionate blow up over previous bug. I made easy to fix bug which was my mistake. But it turned into massive public blow up over work being shitty, us intentionally ignoring his needs, there being tons of bug unusable version (there was literally one bug). Then he wanted massive refactoring to avoid possibility of same bug ... and I made bug in refactoring which led to same blow out, again claiming it was done without care etc.

After that, I really did not wanted to do any changes in that code. I cant guarantee complete lack of bugs in my code. Other people do bugs too for that matter, I dont think I make so much more of them. But, he was under pressure and stress that had nothing to do with me and I became rod for that.

[sgtnoodle]

That sounds miserable. I think, as a reviewer, it's important to keep in mind that most problems have many solutions. You have to be flexible and work with the person that did the hard work of implementing it. Everyone messes up and lets a bug slip through every now and then. That's what multiple layers of thorough testing is for.

On my current team, I'm considered to be the most thorough code reviewer. Folk usually thank me rather than scorn me, though. Some are afraid to send me their PRs not because they fear my feedback, but because they think reviewing their code will take up too much of my time. Reviewing code thoroughly doesn't take much time at all, though, if you make it a habit. If it's a bug, I explain my concern and suggest a fix. If it's a style nit, I link to the relevant style guide section. If it's a suggestion or personal preference, I explicitly say that, write out my rationale, then offer to chat more. If it's a weak suggestion, I tell them right off the bat that I'm fine either way. If it's a strong suggestion, I try to give them an "early out" by suggesting they simply add a // TODO comment. I'll let a coworker get away with murder as long as they leave a TODO comment.

For new team members that send me a PR for the first time, I typically send a message at the start describing what they should expect. That helps a lot, because everyone's first few PRs are going to be rough until they've gotten up to speed on the existing team's expectations.

I will say that there are some engineers that just don't take technical feedback well. When they join an existing team, they can be stubborn and refuse to adapt to the established culture. Instead, they either misinterpret criticism as personal attacks, or get frustrated and insist that the team conform to their preferences right off the bat. Team culture can and should evolve over time, but it requires respect and understanding of the status quo. It's possible for an open minded engineer to join a team, embrace its current culture, then radically change it all over the course of a few months. It's possible for a close minded engineer to not get more than a dozen PRs approved over the course of a year. Not to say that such engineers are good or bad one way or another, but rather folk should seek out projects that are compatible with their personality.

[sgtnoodle]

Also, it helps a lot to hammer out a lot of design and implementation details before writing any code. For any work project I start that I anticipate taking more than a few hours to code, I spend an hour or so writing an "architecture document" which I then distribute to anyone I think will have an opinion about it. That gives folk a place to ask questions and "bike shed" the problem long before I've invested any "artesianal coding energy". By the time the code reviewer looks at the code, they know what it's supposed to do and why it was written that way. As a pleasant bonus, I usually code about 3x faster when I have the document to reference.

I try to get new hires into this habit as early as possible to varying degrees of success. The folk that embrace it tend to be very productive; I don't know if it's causation or just correlation though. I have a slide deck to emphasize function over form, titled "Writing Mediocre Architecture Documents". It's a cult classic hit.

[frfl]

> Writing Mediocre Architecture Documents

Would you be willing to share this slide deck?

My question in regard to this is how do you write an architectural doc if you're exploring the problem space? If it takes 5h to explore, experiment and find the solution. And it takes 0.5h to write the doc. And 0.5h to make the change after you know the solution and have reverted the experimental/exploratory changes?

At that point is the doc necessary as a precursor to the change. By that I mean, do you send it out for feedback first and wait OR do you send it out/add that as a comment on you PR and push up the final change?

Additionally how do you know you're going to get feedback at all from sending out the doc? In my experience people are always busy with their own work and sending them a big chunk of text...well I wouldn't expect a response quickly compared to a short 1-2 line question

[sgtnoodle]

I consider any code that's written to be "speculative" until its design has been vetted by another engineer. By speculative, I mean that there's no expectation that the code will be approved and merged in its present form. An architecture document is simply a good way to get that buy-in. If I write down what I plan on doing in a document, and then solicit feedback on it from a coworker, there's a very good chance that the same coworker will approve the resulting code without anything but bug and style fixes. If you're exploring the problem space, yeah of course go ahead and hack away at some code. If the experiments go exceedingly well and you end up with production quality code, go ahead and throw it in a PR for review. You have to be receptive to "bike shedding" feedback, though, since the PR is the first opportunity you gave for folk to give that type of feedback.

It's also perfectly reasonable to write and distribute a document that effectively says, "I have no idea what the requirements are, but I have an arbitrary idea for how we should proceed anyway." More often than not, your coworkers will help flesh out the requirements, or agree with your arbitrary design decisions. Either way, it's a lot easier to have that type of discussion on a free-form wiki page rather than in a github PR that you already spent 4 hours getting to compile and pass test suites.

Re: soliciting feedback, even verbose architecture documents tend to be relatively easy for your coworkers to digest. It is communication written for human consumption rather than machine consumption, after all. For one thing, you can have a lot of fun with them. One of my recent arch documents had at least a dozen MC Hammer puns in it. I had no trouble getting anyone to read through that. Also, if you give your coworkers the opportunity and they don't follow up on it, it does give you the high road in any eventual PR contention. "I mentioned this a week ago and asked you for feedback on it, why are you making a fuss about it now?" Not that you should be setting up your coworkers like that. You should make a good faith effort to solicit feedback, and keep politely pestering folk until they get to it. Until you get buy-in, any code written is still speculative, even if you tried your best and your coworkers let you down.

It helps to lead by example. If you prioritize giving feedback to coworkers over your own implementation work (reading their documents, doing their code reviews in a timely way, etc.), then your coworkers will tend to notice and reciprocate.

[frfl]

That's very insightful. Appreciate the write up. Thank you.

I noticed the slide deck stuff in a sibling comment, +1

[justanotherguy0]

Is that slide deck publicly available?

[sgtnoodle]

Nah, but here's the content. You'll have to add your own clip art hastily pulled from google image search:

---

What’s the point?

Architecture documents are for you, not for everyone else

* Adds structure to chaos * Makes feedback progressive rather than regressive * You get to tell people to “Read The Friendly Manual”

---

But there’s so much boilerplate!

So don’t fill out the useless parts...

* Delete sections that aren’t relevant * If there’s a better format, just do it that way * Complain to <template owner> if the template is silly

---

The requirements are too ill-defined to write down

I’m sure writing code will solve the problem then. /sarcasm

* This is exactly why architecture documents are helpful * Just barf out some bad requirements and ask for feedback * If no one has any ideas, we’ve got bigger problems

---

I’m the one writing the code, back off!

Software engineers are experts about everything and tend to have opinions.

* Enumerate alternative approaches and politely explain why they are dumb * Detail out the chosen approach * When in doubt, write out function signatures

---

When is enough enough?

When it starts to feel passive aggressive

* Writing code without an architecture doc is speculative * “Wasting” 30 minutes on a doc can save hours in a PR * I’ve never regretted writing an architecture document

---

Super Fun Activity Time

Whatever you’re currently working on, write an architecture doc for it.

(If you already have one, quit wasting time and go write code!)

You have 15 minutes!

[bentcorner]

> where code review evolved into huge micro-management with inconsistent requirements on what good code looks like.

In my org this can be somewhat of a problem (different teams have different code style guidelines for long historical reasons which can make cross-team changes tricky).

One thing that helped a lot with this is publishing these guidelines in a central location, but more importantly we also made bots that reviewed every code review to point out common mistakes that people made, so reviewers could focus on the change itself instead of naming/spacing nits.

[jayd16]

I know I've been testy with a coworker or junior before but its so counter productive to have a workplace with fear. Process can lead to trust and confidence so I try to focus on identifying and improving that (as well as mentoring around risky patterns and such.) Test, reviews, tooling etc. If simple mistakes are effecting the team, its really a process failure.

Fear driven development is toxic.

[kevinmchugh]

Obviously these are people problems. If my next team suffers from your first problem, I might try to ease it by introducing a code formatting tool like prettier. I don't always love the choices those tools make, but I do love eliminating that class of PR comment.

It doesn't eliminate the people problem, but it does limit its scope.