1. 11
  1.  

  2. 9

    It’s s really wordy article for no real reason: took about 3 paragraphs to just state “developers don’t like reviewing large PRs”.

    1. 5

      Also, readers don’t like wall of texts that span 25 screens… ^^ This guy must be writing java entreprise code :p

      1. 0

        I agree it was perhaps too wordy but that TL;DR doesn’t do it justice at all.

      2. 3

        The first change I would make to pull requests is to move away from forks being a required part of the workflow.

        Amen brother! I find the “fork first” model so annoying. I don’t want to maintain a fork, I just want to clone the main repo and submit a change request. I do like the phabricator approach, and sr.ht seems to have a similar “no fork” required approach with the git send-email approach. I personally don’t care about tracking the individual commits in a PR since I feel like a unified PR should be small and just do one thing.

        1. 2

          My ideal workflow for open-source would be git-clone-commit-push. For unauthorized users the server should create a pull request instead of putting the pushed commits onto the branch. The only fork is my local copy. I don’t know any git-server-tool which works like that.

          1. 2

            I think this is more or less what repo.or.cz does with mob:

            https://repo.or.cz/h/mob.html

        2. 2

          PR’s are only a thing because devs can’t manage branches and personal workflow, and would rather lean on the tool to solve their social problems.

          Look, if you work in a team and are well organized, and the team is communicating well within itself, you don’t have to fret about PR’s. You can just let the devs have their own branches, and when necessary, have a merge party to produce a build for testing.

          Alas, not all projects can attain this level of competence. PR’s are there to allow community, even if folks can’t organise well enough to know each other and how to contribute to each others work in a positive flow.

          For many projects, a rejected PR is just as valuable as an auto-merge. It gets people communicating.

          1. 2

            One potential problem with giving each dev their own branch and merging all branches at once to make a build is that build has a lot of potential changes and when something inevitably goes wrong it isn’t always clear what exactly caused it to break. Good communication and code quality can mitigate issues with integrating the code, but I don’t think it completely eliminates it. If the alternative is releasing a build with multiple PRs anyway then this might not be a problem, but if your alternative is releasing a build with every single change then its a distinct disadvantage.

            1. 2

              Why would you want to have a merge party where you merge in a whole bunch of stuff at once rather than reviewing and merging smaller changes one at a time?

              1. 1

                Maybe because your team is productive.

                Because you’re pushing features forward, trust your fellow devs, and everyone is working well enough that it doesn’t matter - and it means that features can be tested in isolation. Plus, its very rewarding to get a branch merge done, you suddenly get a much bigger and better app for the next round of work ..

                1. 1

                  Not in the long run and not if your code is in production. One of the goals of review process is to get the others in the team acquainted with the changes, so they could support that in the future, when the author goes on a vacation or leaves the company. Merge parties cram way to much information for a purposeful comprehension. Remember, coding is mainly joint activity of solving business problems, and its product will require support and maintenance.

                  1. 1

                    Our Merge parties include team review, so .. not really encountering the issues you mention.

                    1. 1

                      How big are your change requests, how long do these reviews last?

                      1. 1

                        Weekly, takes a day for the team, and then we start the 3-day tests ..

                        But look, whatever, not everyone’s use case is the same, The point is, scale according to your needs, but don’t ignore the beauty of PR’s as a mechanism, when you need them. If you don’t need them, do something else that works.

            2. 1

              You can just let the devs have their own branches, and when necessary, have a merge party to produce a build for testing.

              Is that really a thing? I’ve never been on a team that did that, and it sounds like a mess (to put it lightly). I can’t imagine that it would scale well.

              1. 1

                For sure. I do it with the 3 other devs in my office space. We just tell each other ‘hey, I’m working on branch’, then when the time is right, we all sit together and merge.

                I mean, its probably the easiest flow ever.

                If you don’t do this, I wonder why? I guess its communication.

                1. 3

                  I do it with the 3 other devs in my office space.

                  That makes sense: the “3” and “in my office space” are a particular set of constraints. If you’ve got a system worked on by more people, and distributed across locations, that doesn’t scale in quite the same way, I think.

                  1. 1

                    Get back to me when you’ve tried it with more than 10 people on the team or a project with more than 500k lines of code…

                    1. 1

                      Yeah, works just as fine at that scale too. Key thing is: devs communicating properly.

              2. 1

                I appreciate more people discussing this. My observations tell me that PRs are fine for open source development, where distributed development teams are working at different times and at different paces. I think there are more optimal workflows for development teams working at the same company. That being said, things were pretty bad before PRs. PRs have created a sane workflow that most companies are better off for having adopted.

                There is no singular optimal workflow. If you want to optimize the workflow for your team, then you have to build a workflow that takes into account:

                • How many people are contributing to the repository? A 3 person development team requires different controls than a company with 10,000 engineers all contributing to a monorepo.
                • How often is the code in the repository put into production? Some companies release hundreds of times per day, while others release once per year.
                • Does the repository have good test coverage? If the code has sufficient tests, then changes can be introduced with less risk of breaking things. Note: this may not help design related issues.
                • How established the repository? Are there existing conventions that people know to follow? Are massive changes being made that often cause merge conflicts?

                I am sure there are other important points to consider that I am missing. The above is a good start though and I think this should be a discussion that teams have. If the answer is to stick with PRs, then that is fine. Maybe there is some room for a little experimentation though.

                1. 1

                  I feel like a good vcs tool is now as scared as containerization and orchestration tool in the past. Its waiting for some good open source initiative thats is viable and scalable to become a “trend”. Then it stops become an enterprises closed source tool and get adopted widely

                  1. 1

                    Scared?

                    1. 1

                      I think they mean “sacred”

                      1. 1

                        maybe a typo for “scarce”