1. 4

  2. 4

    I have a suspicion that in some contexts being even more aggressive would be optimal:

    • by default, PR are merged by the author without blocking for the review
    • after PR is merged, it is assigned a reviewer who does a “post-review” by sending a follow-up PR
    • PR author can opt-in into pre-review if they feel like they need input.

    This requires several unlikely factors though:

    • strong testing&reliability culture, so that there’s a reasonable degree of certainty that a random PR can’t catastrophically break things silently
    • high cultural cohesion, so that PR/follow-up converge to agreed-upon perfect code, rather than oscillate between two different styles of doing things
    • slow team growth, so that the cultures could be maintained.
    1. 1

      this all seems like an incredibly complicated way to deal with a lack of trust between members on a team/project. i’d argue that you don’t need any of this and should instead focus on why there is a lack of trust to begin with. this isn’t a new problem,right?

      for example, a new person joins the team/project. maybe don’t immediately give them access to merge to main/master, and require that they submit all changes as merge requests for review first.

      another example, you have someone who plays loose and fast with merging stuff without review. maybe revoke their merge access to main/master immediately, talk to them about it, and work out a plan to address the problem. if they continue to ignore the process, then don’t give them access to merge and consider taking other action (fire? remove from project? the list goes on…)

      am I missing something?

      1. 2

        Are you suggesting that an even better process is to not do reviews at all, and just trust people to do a good job themselves?

        I think that would be incredibly productive medium term, but less so long term. I do think that “review every change” is useful:

        • second pair of eyes often makes the code better/reveals bugs.
        • reviews help to spread knowledge about how a component works, they open up siloes of expertise.
        • projects written in “one voice” are easier to work with, and reviews allow for the discovery and formulation of a common voice
        1. 1

          No I never said to not do reviews. What I mean is, if you have a process and you can’t trust others to follow it, rather than rely on complicated, janky automation and github features, focus on the core problem.

          1. 1

            Hm, did you perhaps meant to comment on the story, rather than on my comment? I definitely do not suggest more automation. Otherwise, it seems like I am misunderstanding something :)

            1. 1

              ugh, yes I did… sorry for the confusion :(