1. 15
  1.  

  2. 4

    This seems like the Gerrit approach.

    Every day I have to use Github, I long for Gerrit’s embrace.

    1. 3

      I think this post sort of belabors the point: small changelists are better than long ones, and it’s easier to manage small PRs if you have better tooling for it.

      I disagree with the “one local commit is one diff” argument, and although my company uses phabricator, we don’t use it that way. Sometimes a commit is only part of an idea, or doesn’t include tests, and that can still make sense as a local commit–with that said, it probably won’t make sense as a commit on master, and you should take advantage of the diff to collate different pieces that will become a single commit eventually.

      1. 2

        (I’ve never used Phabricator, but I’ve opened 1000+ PRs on GitHub, so you can guess where my biases come from and take my grouching below with a bucket of salt.)

        I found it a bit hyperbolic… Creating a new branch in Magit is 4 keystrokes + the length of your branch name. I use a ticket number, so usually less tan 10 keypresses. Creating a PR (also from within Magit) is another 5 if you wrote a good commit message that can be re-used as a PR description. This, rather than having to change to a terminal (let’s say 1 keypress) typing phabricator + RET (another 12 keypresses) and we’re about even, if you can select the commit you want to push to review with the remaining 2 keypresses. (You would probably have an alias that is shorter than phabricator. I get it. I can be hyperbolic too!)

        I did like the “Coding as a queue” section, and admit I found that intriguing. I think I can see how this “Stacked diffs” workflow would work well in a Monorepo, in particular. Many places, including ours, don’t have that though: we have many (perhaps too?) small repos. Here the benefit becomes less obvious to me.

        I have also been in the situation that I have several PRs that depend on each other. But I am not sure I understand the concrete benefits Stacked Diffs offers here. The bugfix (or other dependent change) still needs to be reviewed, and sometimes you need the context of that bug fix to understand some of the other changes, so it’s useful to have part of the PR IMO. I normally create that as part of a PR, but then create a new branch off master and cherry-pick those fixes to that for a new PR, once things build. Merging that first, I can rebase my original change and it’ll be clean.

        One benefit of PRs that I didn’t see listed is that for drive-by changes I can “outsource” testing to the CI tool rather than having to run tests locally for every change. CI runs all tests, so as I explore code and come across something I want to fix as an unrelated change I can quickly create a new branch, commit that code, push it, and get back to my work while the CI tool runs tests to make sure I don’t miss anything. PRs are also a great way to compare and contrast different (but incompatible) approaches to fix a problem.

        Another thing I like about PRs, as a reviewer, is that you can easily switch to see more context. I’ve never used it, so I’m not sure if Phabricator has some extra smarts compared to “patches on a mailing list” here, but I’m often not able to mentally add the context of surrounding code. Being able to expand the section immediately below / above the diff in a PR is often invaluable.

        That said… I am interested in learning about anything that can improve my workflow. I just wish there was a more meaty argument than “creating a branch in git is hard and takes too long” in the post.

        1. 2

          I’m not sure if this applies to Mercurial as well, where we don’t manage branches like in git (branches are long lived, like per developer or supported major revision). I find hg histedit really good at supporting a workflow where you manage you changes and then “rewrite history” to have clean set of commits to send for review. I like how the article goes into the detail of the workflow, though.

          1. 1

            OK, this sounds very intriguing, if only for being so disconnected to how I/we usually work; and I want to dig into it when I’m fresher, but if someone could ELI5 to after-hours-me, that’d be splendid.

            As for PRs, I did find myself longing for better tools to tackle larger Pull Requests than what the GitHub UI has to offer. Mainly “saving progress” somehow, or marking individual files as “read”.

            1. 3

              As for PRs, I did find myself longing for better tools to tackle larger Pull Requests than what the GitHub UI has to offer. Mainly “saving progress” somehow, or marking individual files as “read”.

              There’s a bunch of commercial services that do this, I think https://reviewable.io/ is one. A past client of mine uses Reviewable.

            2. 1

              FreeBSD uses phabricator. I couldn’t figure out how to only send the master commits I want using the CLI tool (arc) so I use branches anyway, and send the branch as a diff to phab :D

              1. 1

                I will fully admit that other than always having a clean git bisect, I don’t quite understand the merits/differences of this workflow at the moment.

                1. 3

                  For one, you can use the version control history as a piece of documentation, where each change comes with a useful commit message, and does only one thing.

                  1. 2

                    does only one thing

                    This also makes operations such as rebasing and cherry-picking much easier.