Live data from Hacker News

GitHub Stacked PRs

github.github.com

511–520 of 548 posts

Re: GitHub Stacked PRs

#511
post #359

Earlier quoted context omitted.

You must belong to the club of folks who use hashmaps to store 100 objects. It's amazing how much we've brainwashed folks to focus on algorithms and lose sight of how to actually properly optimize code. Being aware of how your code interacts with cache is incredibly important. There are many cases of using slower algorithms to do work faster purely because it's more hardware friendly. The reason that some more modern…

> You must belong to the club of folks who use hashmaps to store 100 objects. Apparently I belong to the same club -- when I'm writing AWK scripts. (Arrays are hashmaps in a trenchcoat there.) Using hashmaps is not necessarily an indictment you apparently think it is, if the access pattern fits the problem and other constraints are not in play. > It's amazing how much we've brainwashed folks to focus on algorithms an…

My core complaint is that folks repeat best practices without understanding them. It's simple to provide API semantics that appear like a map without resorting to using hashmap. I fear python style development has warped people's perception for the sake of simplifying the lives of developers. And all users end up suffering as a result.

Re: GitHub Stacked PRs

#512

Earlier quoted context omitted.

Why not use gerrit? It's pretty similar.

is it? it's insanely ugly and interactions seem bizarre

Previous iterations have been a bit dated in terms of UI, but modern versions are pretty good. What interactions are bizzare? Leaving comments, approving a change and running presubmit tests are all pretty straightforward.

Re: GitHub Stacked PRs

#513

Earlier quoted context omitted.

The question is, why are you not just merging them into main as you go? It's a bit of a smell when you "need" to merge branches into branches. It shows a lack of safety and ease in deployments, which is the real problem to solve IMO.

Because sometimes there are changes that need to land as all or nothing.

Stacked PR’s… don’t do that though? They’re just PR’s. You can merge just the first one in the stack, and now it’s not “all or nothing”. Reading the docs, I don’t see a way to signal that the PR’s must all merge together.

Because the most natural way of saying “these changes need to land atomically” is called a branch, and landing it atomically is called a “merge”. But I guess GH’s UI sucks for reviewing large changes, so we’re stuck having to make each change independently mergeable and pass tests (likely disabling dead-code lints, etc) just to work around this limitation. Sigh.

At least when I actually do want changes to be mergeable in a stack, I now have a better UX for having folks review them.

Re: GitHub Stacked PRs

#514

Earlier quoted context omitted.

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?

> so the diff in a PR shows just the relevant changes from the specific branch 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 c…

When you're using the UI to open a PR, if you set the base to a branch that has an open PR there'll be an "Add to Stack" option: https://github.github.com/gh-stack/guides/ui/#step-2-create-...

Stacks require users to explicitly indicate that they are opening a PR that should be part of a stack.

Re: GitHub Stacked PRs

#515

Earlier quoted context omitted.

A unit of change is a commit. I have no idea why you'd think a PR is a unit of change.

Does your org approve specific commits or PRs overall?

Looking at individual commits is part of the PR review process.

Re: GitHub Stacked PRs

#516

Earlier quoted context omitted.

Sure, that's possible. I can also use the GitHub app and use a Git abstraction where I don't have to understand Git at all. The point is that I want to use Git, a tool and skill that is portable to other platforms.

> The point is that I want to use Git, a tool and skill that is portable to other platforms. You want to use git. Most people around you want to get things done.

lol and github is the primary obstacle to that in today's software engineering

Re: GitHub Stacked PRs

#517
post #499
post #460

Earlier quoted context omitted.

Mercurial's model is different from Git that these things you list does not make sense there. Rebase does not make sense in Mercurial because it has the concept of fixed branches. A commit is permanently linked to the branch on which it was made. So you are supposed to use merges. Same with force-pushing.

I'd like to fill up some inaccuracies in your response: - rebasing in Mercurial simply means chopping a subtree off of the history and re-attaching it to a different parent commit. In that sense, rebasing is a very useful and common history-rewriting operation. In fact, it's even simpler and more powerful/versatile than in git, because mercurial couldn't care less if the sub-tree you are rebasing belongs to a branch…

> rebasing in Mercurial simply means chopping...

Sure, but since commits have a branch attribute attached to them, "rebasing" does not appear to be "first class". It is something that has to be bolted on with an extension.

> because mercurial couldn't care less if the sub-tree you are rebasing belongs to a branch or not

IIUC Git also does not care much about the rebase target being a "branch".

I agree that Mercurial provides more value out of the box than git because it preserves branch info in commits.

I can live with Git because Git is "enough" if used carefully and after coming to terms with the non-intutive UI.

Re: GitHub Stacked PRs

#518

Does it fix the current UX issue with Squash & Merge? Right now I manually do "stacked PRs" like this: main If PR B merges first, PR A can merge to main no problems. If PR A merges to main first, fixing PR B is a nightmare. The GitHub UI automatically changes the "target" branch of the PR to main, but instantly conflicts spawn from nowhere. Try to rebase it and you're going to be manually looking at every non-conflic…

> Try to rebase it and you're going to be manually looking at every non-conflicting change that ever happened on that branch, for no apparent reason

My "fix" is to do an interactive rebase of PR B on main and drop all of PR A's commits from PR B in the process.

I remember seeing a way to do this automatically, but it requires an option that I never remember. IMO this is kind of the issue with git: a lot of improved workflows sit behind some flags that most people never learn. Interactive rebases work for me because they are one primitive, always working in the same way.

Re: GitHub Stacked PRs

#519
post #92

Earlier quoted context omitted.

Conflicts spawn most likely because PR A was squashed, and once you squash Git doesn't know that PR B's ancestors commits are the same thing as the squashed commit on main. No idea if this feature fixes this. Edit: Hopefully `gh stack sync` does the rebasing correctly (rebase --onto with the PR A's last commit as base)

> Conflicts spawn most likely because PR A was squashed, and once you squash Git doesn't know that PR B's ancestors commits are the same thing as the squashed commit on main. Yeah, and I kind of see how git gets confused because the squashed commits essentially disappear. But I don't know why the rebase can't be smart when it sees that file content between the eventual destination commit (the squash) is the same as t…

Even if you tried something like that it will eventually break when other commits are added to main that are not present in PR B, even if those commit don't conflict with neither PR A nor PR B changes.

Re: GitHub Stacked PRs

#520
post #315

Earlier quoted context omitted.

Git-lfs exists for a while now. Does that fix your issue? Or do you mean that it doesn’t support binary diffs?

Git LFS is a gross hack that results in pain and suffering. Effectively all games use Perforce because Git and GitLFS suck too much. It’s a necessary evil.

We use git-lfs quite contentedly but we don’t require diffs on binaries. What pain and suffering are you eluding to specifically?
Post reply on HN