3 ms·

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

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 6y ago
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.
- justanotherguy0 6y ago
  Is that slide deck publicly available?
  - sgtnoodle 6y ago
    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!
- frfl 6y ago
  > 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 6y ago

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.

justanotherguy0 6y ago
Is that slide deck publicly available?
- sgtnoodle 6y ago
  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!
frfl 6y ago
> 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