1. 7
    1. 3

      A lot of the PRs I’ve received on projects I maintain are, quite frankly, not very good. It’s often sloppy half-arsed and for stuff I don’t really care about myself (if I had, I would have probably made it myself already).

      I do the review and merge because it will make the project more useful in general, but at the end of the day there’s only so many hours in a day and I need to prioritize. To be honest I typically don’t really want to spend a whole lot of time on code I don’t care about. So certainly in the context of open source I tend to shift the investment of time to the PR author, rather than stepping up myself especially if they did a sloppy job (it’s different at work).

    2. 2

      I think that this model isn’t too bad when working with peers. When working in a professional environment where there might be a skills gap, having a (usually more skilled) reviewer just “fix all the stuff” isn’t a great experience. Kinda important for less skilled people to do things on their own sometimes!

      Now, one could sidestep this with pairing. Personally I feel like there’s value in having people venture out a bit more on their own as well (gives them an opportunity to find new paths that the team might not know about), and in those cases having kinda vague responses to stuff is kinda the point.

      You can definitely do “vague unsure reply to PR” better or worse. Saying “hmm dunno about this” is not very actionable so just leaves people hanging. The reviewer should at least feel responsible for any work they’re asking their teammates to do.

Stories with similar links:

  1. Reviewing Pull Requests (2019) via fcbsd 12 months ago | 4 points | no comments