10 ms·

As someone who used phabricator and mercurial, using GitHub and git again feels like going back to the stone ages. Hopefully this and jujutsu can recreate stack

• kardianos 6mo ago
  I continue to use gerrit explicitly because I cannot stand github reviews. Yes, in theory, make changes small. But if I'm doing larger work (like updating a vendored dep, that I still review), reviewing files is... not great... in github.
  • tcoff91 6mo ago
    Most editors have some kind of way to review github PRs in your editor. VSCode has a great one. I use octo.nvim since I use neovim.
    • nine_k 6mo ago
      Can these tools e.g. do per-commit review? I mean, it's not the UI what's the problem (though it's not ideal), it's the whole idea of commenting the entire PR at once, partly ignoring the fact that the code in it changes with more commits pushed. Phabricator and even Gerrit are significantly nicer.
      • dathanb82 6mo ago
        Unless you have a “every commit must build” rule, why would you review commits independently? The entire PR is the change set - what’s problematic about reviewing it as such?
        • steveklabnik 6mo ago
          In stacked diffs system, each commit is expected to land cleanly, yes.
          • verst 6mo ago
            But isn't that why you would squash before merging your PR? If you define a rule that PRs must be squashed you would still have the per commit build.
            • steveklabnik 6mo ago
              Squash merge is an artifact of PRs encouraging you to add commits instead of amending them, due to GitHub not being able to show you proper interdiffs, and making comments disappear when you change a diff at that line. In that context, when you add fixup commits, sure, squashing makes sense, but the stacked diffs approach encourages you to create commits that look like you want them to look like directly, instead of requiring you to roll them up at the end.
        • riffraff 6mo ago
          There's a certain set of changes which are just easier to review as stacked independent commits. Like, you can do a change that introduced a new API and one that updates all usages. It's just easier to review those independently. Or, you may have workflows where you have different versions of schemas and you always keep the old ones. Then you can do two commits (copy X to X+1; update X+1) where the change is obvious, rather than seeing a single diff which is just a huge new file. I'm sure there's more cases. It's not super common but it is convenient.
          • strokirk 6mo ago
            Wouldn’t it be easier to do those as stacked PRs then?
        • Sebb767 6mo ago
          > Unless you have a “every commit must build” rule, why would you review commits independently? Security. Imagine commit #1 introduces a security vulnerability (backdoor) and the features. Then #2 introduces a non-obvious, harmless bug and closes the vulnerability introduced in #1 [0]. At some point, the bug will surface and rolling back commit #2 will be an easy fix, re-introducing your bug. Alternatively, one of the earlier commits might, for example, contain credential dumping code. Once that commit is mainlined, CI might either automatically run on it or will be able to be run on it since it's no longer marked as unsafe PR. [0] Think something like #1 introduces array access and #2 adds a bounds-check in a function a layer above - a reviewer with the whole context will see the bounds check and (possibly) consider it fine, but to someone rolling back a commit the necessity will not be obvious.
  • adityaathalye 6mo ago
    Same team, and a rare hill I'm willing to die on. Rant incoming... Boy do I hate Github/Lab/Bucket style code reviews with a burning passion. Who the hell loses code review history? A record of the very thing that made my code better? The "why" of it all, that I am guaranteed to forget tomorrow morning. Nobody would be using `--force` or `--force-with-lease` as a normal part of development workflow, of their own volition, if they had read that part of the git-push manpage and been horrified (as one should be). The magit key sequence for this abominable operation is `P "f-u"`. And every single time I am forced to do it, I read "f-u" as it ought to be read. Rebase-push is the way to do it (patch sets in Gerrit). Rebase-force-push is absolutely not. You see, any development workflow inevitably has to integrate changes from at least one other branch (typically latest develop or master), without destroying change history, nor review history. Gerrit makes this trivial. It's a bit difficult to convey exactly why I'm so rah-rah Gerrit, because it is a matter of day-to-day experience of - Well, a single commit of a few lines to maybe a hundred lines *is* the correct unit of code review, rebase, revert etc. Manually "Sizing PRs" to that review context size is utter BS. I have better things to do in life than to book-keep PR sizes. Make a single well-contained, revertible commit. Then keep making those. And now you have a commit history that is clean, that you can merge, bisect, and bulk-revert at will. Octopus merges are a good thing. `git-log` is *designed* to let us view changes in any sequence we wish, *including* the so-called "linear" history. `git log --online`. - Trivial for committer to send up reviews-preserving rebase-push responses to commit reviews (NO force-push, ever --- that's an "admin" action to *evict* / permanently wipe out disaster scenarios such as when someone accidentally commits and pushes out a plaintext secret or a giant blob of the executable of the source code etc.). - Fast-for-the-reviewer, per-commit, diff-based, inline-commenting code reviews. - The years-apart experience of being able to dig into any part of one's (immutable) software change history to offer a teaching moment to someone new to the team. ... to name a few key ones. (edit: add point about review size)
    • adityaathalye 6mo ago
      Slapping this "stacked diff" business on top of something so broken as Github/lab/bucket is a concrete example of... https://en.wikipedia.org/wiki/Lipstick_on_a_pig https://en.wikipedia.org/wiki/Lipstick_on_a_pig
• smallmancontrov 6mo ago
  I'm so glad git won the dvcs war. There was a solid decade where mercurial kept promoting itself as "faster than git*†‡" and every time I tried it wound up being dog slow (always) or broken (some of the time). Git is fugly but it's fast, reliable, and fugly, and I can work with that.
  • Leynos 6mo ago
    I just used it because I preferred the UX.
  • forrestthewoods 6mo ago
    Mercurial has a strictly superior API. The issue is solely that OG Mercurial was written in Python. Git is super mid. It’s a shame that Git and GitHub are so dominant that VCS tooling has stagnated. It could be so so so much better!
    • awesome_dude 6mo ago
      Whatever your opinion on one tool or another might be - it does seem weird that the "market" has been captured by what you are saying is a lesser product. IOW, what do you know that nobody else does?
      • jrochkind1 6mo ago
        Welcome to VHS and Betamax. the superior product does not always win the market.
        • Per_Bothner 6mo ago
          Not always, but in this case the superior product (i.e. VHS) won. At initial release, Beta could only record an hour of content, while VHS could record 2 hours. Huge difference in functionality. The quality difference was there, but pretty modest.
          • 6mo ago
• calebio 6mo ago
  I miss the Phabricator review UI so much.
  • montag 6mo ago
    Me too. And I'm speaking from using it at Rdio 15 years ago. Nothing since (Gerrit, Reviewboard, Github, Critique) has measured up...
    • Rodeoclash 6mo ago
      Thanks for your work on Rdio. I miss it. Were you around when that guy managed to spam plays to get fake albums to the top of the charts?
  • sam_bristow 6mo ago
    What does Facebook use internally these days. I'm amazed that the state of review tools is still at or behind what we had a decade ago for the most part.
    • ivantop 6mo ago
      It’s still phabricator
      • sam_bristow 6mo ago
        Any idea if their internal version has improved dramatically since they stopped maintaining the public version?
        • kqr 6mo ago
          I don't think they ever maintained the public project. Priestly spun off a company to do that.
        • xixixao 6mo ago
          It has improved massively by 2023 when I left.
• eru 6mo ago
  Oh, phabricator. I hated that tool with a passion. It always destroyed my carefully curated PR branch history. See https://stackoverflow.com/questions/20756320/how-to-prevent-phabricator-from-eating-my-commit-history https://stackoverflow.com/questions/20756320/how-to-prevent-...
  • illamint 6mo ago
    Good. That's the point.
    • eru 6mo ago
      The point of what? I hope they fixed phabricator in the meantime.
      • dbetteridge 6mo ago
        The point is the main branch reflects the "units" of change, not the individual commits to get there. One merged pr is a unit of change, at the end of the day the steps you took to produce it aren't relevant to others. My opinion of course, I'm open to understanding why preserving individual commits is beneficial
        • eru 6mo ago
          You can get what you want from `git log --first-parent` without having to toss out information. See how the Linux kernel handles git history to see a good example of non-linear history and where it helps. They use merge commits, ie commits with more than one ancestor, all the time.
        • saagarjha 6mo ago
          A unit of change is a commit. I have no idea why you'd think a PR is a unit of change.
          • zip1234 6mo ago
• nerdypepper 6mo ago
  tangled.org supports native stacking with jujutsu, unlike github's implementation, you don't need to create a new branch per change: https://blog.tangled.org/stacking/ https://blog.tangled.org/stacking/
• choi0330 6mo ago
  You should definitely try out https://github.com/hokwangchoi/pilegit https://github.com/hokwangchoi/pilegit. It's platform-agnostic and I use for my workflow with Phabricator, Github, Gitlab and Gitea. No learning curves for cross-platform operations!