Doc Avid Mornington

Not actually a doctor.

  • 0 Posts
  • 7 Comments
Joined 1 year ago
cake
Cake day: July 5th, 2023

help-circle

  • I get the feeling you feel like I was somehow calling you out. I want to clarify the the intent of my message was more in the spirit of "wow must be nice" than "you're making that up". But also I'm just interested in how different your experience is from mine.

    Who said anything about only requiring 1 reviewer?

    I must have misunderstood. You said "If no one has reviewed your change within 24 hours you are allowed to approve it yourself." To me, that sounds like, after 24 hours of no review, one self-approval is considered sufficient. That, in turn, seems to imply that before 24 hours, one non-self-approval is probably sufficient, no?

    You should try working for a healthy team where everyone takes collective responsibility and where the teams progress is more important than any one person's progress.

    I've had team members in the past who are very self-focused, they tend to close a lot of tickets and look good, then get promoted out, leaving an unmaintainable mess behind. Allowing that is generally a failure of leadership. But right now, that's not our problem, and what you describe is pretty much how we operate.

    I'd love to work on a team where everybody took code review a lot more seriously, believe me, it'd be nice, but my team does generally get everything approved, with at least two non-self approvals, in under 24 hours. If something is getting ignored because people are busy and it's a large change because we aren't perfect, and there is some reason to get it in soon, it just takes a quick request on Slack to get the needed attention.

    What I found surprising about your description was more that the potential of a self-approval coming up would, in itself, get people's attention, rather than somebody reaching out personally and asking for a review.

    Our big weakness is review quality, not quantity. It's crazy the number of times I look at something and see the two or three approvals already, start going through it, and find issue after issue. I see that on other teams as well, where there's usually only one or two devs who ever really make any comments on a review, it seems to be very common.





  • To be fair, it said "an enormous amount of code", not "your entire app", but yes, the ability to add unexpected new features or make focused changes without touching more than a minimal amount of existing code is a very good smell metric of code quality. The problem is that for every dev who understands how to program like that, there are at least five, probably more like ten who don't, which means most of us are working on teams that produce a blend of clean code and, as you say, dog shit, so the feature request that requires stirring up all that shit is out there waiting for us, like it or not. The best we can do, when it hits, is try to at least improve all the shit that we touch in the process. Maybe some of it can become compost, I dunno, the metaphor breaks there, gonna have to refactor the metaphor.