Earlier quoted context omitted.
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!
Maybe forgejo has a shot?
GitHub Stacked PRs
431–440 of 548 posts
Re: GitHub Stacked PRs
#432Earlier quoted context omitted.
Networking effects are significantly strengthened by necessary user buy in. VC is hard, and every tool demands its users to spend a non-significant amount of time learning it. I would guess the time to move from black magic to understanding most of git is ~100h for most people. The thing is, to understand which one is actually better, you would have to give the same amount of investment in the second tool, which is n…
A little over a decade ago, with only svn experience, I tried both mercurial and git. There was something about how mercurial handled branches that I found extremely confusing (don't remember what), while git clicked immediately - even without reading the manual. So at least for me, git was clearly better.
Re: GitHub Stacked PRs
#433Earlier quoted context omitted.
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!
> The issue is solely that OG Mercurial was written in Python. Are we back to "programming language X is slow" assertions? I thought those had died long ago. Better algorithms win over 'better' programming languages every single time. Git is really simple and efficient. You could reimplement it in Python and I doubt it would see any significant slowness. Heck, git was originally implemented as a handful of low level…
Yes we are? The slow paths of mercurial have been rewritten in C (and more recently in Rust) and improved the perf story substantially, without taking away from the wild modularity and extensibility hg always had.
Re: GitHub Stacked PRs
#434There’s a startup callled Graphite dedicated to stacked PRs. I have been using them for a while now I always wonder why github doesn’t implement something similar to this. I probaly will try and switch to GitHub to see if it works flawlessly
Yep, very happy with graphite at work.
Re: GitHub Stacked PRs
#435Earlier quoted context omitted.
> - the first PR in the stack creates a search index. > - the second one adds a search API handler. > - the last few do the UI. So you're saying you're going to merge (and continuously integrate, perhaps to production) a dangling, unused search index, consuming resources with no code using it, just to make your review process easier? It's very depressing that review UX is so abysmal that you have to merge features be…
This is not just about the UI, it's about the mental model and management of the changes. Just covering the review process: Yes, you can structure your PR into 3 commits to be reviewed separately. I occasionally structure my PRs like this - it does help in some cases. But if those separate parts are large, you really want more structure around it than just a commit. For example, let's say you have parts A, B and C, w…
Why? I reject the notion that large commits should be intrinsically hard to review.
GitHub already has the concept of "code owners", which are people who have ownership/review responsibility over slices of the codebase, based on globs/pattern matching. But they don't implement the other half of that, which is that a reviewer should be able to see a projection of a given PR, which matches the part of the repo they're the owner of.
There. That solves the entire problem of "this is too big, I can't look at all of it" (because your code ownership says this is the chunk of codebase you say you care about), and if that still isn't sufficient, there's a zillion UI features GitHub could add that they simply don't. Why can't I "focus" on a subset of the changes during review, in a way that helps me ignore unrelated discussions/changes? That is, even if I'm not code owner of the `frontend/` folder, why isn't there a UI affordance that says "let me focus on changes inside `frontend/` and ignore discussions/etc for the rest"?
> By using stacked PRs instead of just separate commits, I can add more commits to the individual PRs
Or you could just add commits to the PR, and if GitHub got the damned UI right, it would be natural to see the "slice" you care about, for all the new commits. Having to rearrange commits into separate PR's and slice-and-dice followup changes to file them into the right PR unit, is (to me) a workaround for how shitty GitHub's review UX is. It really shouldn't be this way.
> Then apart from the review process, there are lots of advantages to keeping changes small [...]
I agree with you on most of these points, but the decision to land smaller changes earlier should be made based on things like "let's get early feedback behind a feature flag" or "let's see how this chunk behaves in production so we can validate assumptions", or "let's merge what we have now so to cut back on conflicts", etc. That's all fine. But I'm vehemently opposed to being required to slice up my changes this way, just to work around a terrible review UI.
Personally, I review code in my development environment GitHub's UI is nonsensically terrible to read code. I could go on for hours about this[0], but when looking in my IDE I can drill into a subfolder and look at the diffs there. I can click and follow symbols. I can look at the individual diff history for any wildcarded subset of the repo, and see how the change was broken into commits. If I'm typing up some feedback to say "try doing it this way instead", I can actually try it myself first to make sure I'm not suggesting that someone do something that doesn't even compile.
And GH's discussion UX is by far the worst part of all of it. If you have a thread of discussions around a line of code, then wake up the next morning and want to see what new comments have been added? Good luck. Your best bet is to check your email inbox, because the comments are actually shown to you there. Using GitHub's "inbox" feature? All that is is a link to PR's you have to look at, with no hints at "why" (it could be a CI run finished for all you know.) Good luck figuring out "why" a PR is on your list. Did someone @-mention you? Who knows. So, find the blue dot next to the PR, click it, and then figure out for yourself what changed since the last time you looked. No, you can't just scroll and find it because GitHub hides half the discussions by default. So you have to go and expand all the collapsed sections to hopefully find that conversation you were having yesterday. But oh, you can only find it in the diff tab. So you click that, but the relevant file is collapsed by default ("Large diffs are not rendered blah blah"), so then click that. Then you may find that discussion.
Contrast this to a mailing list. The discussions are... discussion threads. You pick up where you left off. People's comments are right there in your inbox, newest one on top (or whatever your preference is.) You get notified when there's a new message, and when you tap the notification, it's the actual message, not some link to the PR that makes you click 6 more things to maybe find the message that just happened.
[0] like how the first thing you have to do when opening up the changes tab is ctrl+f search for "Large diffs are not rendered by default" to find the actually-important diffs that are not shown to you because GitHub's backend can't scale to rendering large diffs without friction. Countless times I've been burned by approving a PR because I don't see it making a change to some functionality, only to find out it actually did make said change, but GitHub just decided not to show me it. Seriously, the "large diffs" are the most important ones, and those are the ones you don't see without extra clicks. The mind boggles.)
Re: GitHub Stacked PRs
#436And it doesn't even rebase and merge correctly with fast-forward if there it's a clean set of commits! https://github.com/orgs/community/discussions/5524
Re: GitHub Stacked PRs
#437Since I can still login, is there a web API (using CURL and some identifying session token I could retrieve from my login) I could use to actually do "something". For instance, be involved in the issues of some project?
(for me, that would be mostly valve stuff on linux based OSes)
Any pointers?
Re: GitHub Stacked PRs
#438Earlier quoted context omitted.
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
A unit of change is a commit. I have no idea why you'd think a PR is a unit of change.
Re: GitHub Stacked PRs
#439I thrive on stacked PRs but this sure seems like a weird way to implement support for it. Just have each branch point to their parent in the chain, the end. Just native Git. I've been longing for better GitHub support for this but the CLI is not where I need that support: just the UI.
The CLI is completely optional, you can create stacked PRs purely via the UI. Also the rationale for having a chain of branches pointing to each other was so the diff in a PR shows just the relevant changes from the specific branch, not the entire set of changes going back to the parent/trunk. Curious how you're thinking about it?
That's exactly right.
> you can create stacked PRs purely via the UI
How?
I see from the docs https://github.github.com/gh-stack/introduction/overview:
> When a pull request is part of a stack
How does GitHub determine if a PR is part of a stack? Is it automatically detected so that I don't need to adjust my tooling that already creates chained PRs?
Re: GitHub Stacked PRs
#440Earlier quoted context omitted.
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.
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?
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.