14 comments

[ 3.5 ms ] story [ 38.8 ms ] thread
Any chance you have a post sharing your prompt string? It looks quite useful.
I only found out that feature by accident, I agree that its a great way to ensure the code you're talking about is immutable in a pull request.
(comment deleted)
If I recall correctly, it is possible to modify a pull-request by deleting the "active" commit of the pull request on github and adding something else, i.e. if I do something like rebase -i and then a push -f (which I can, because, after all, it is my own repository I am pushing to.)
The point of this article was that you can use a SHA1 in the pull-request dialog. Because no-one can change a SHA1, it's pinned to that specific commit.
This is a metaphor bug in the UX. In git's original terminology a "pull request" (a request from a subsystem maintainer to pull new stuff -- usually from a branch named something like 3.5-staging-for-linus or whatever) is different from a "patch set" (a single submission of code from anyone, often via email with "PATCH n/m" subjects).

Github treats them essentially the same, which is just wrong. It's the patch set that is the unit of code review. It's just plain never correct to review a "branch", you review specific suggested changes in isolation. The idea of pulling branches is to merge already-reviewed code from different trees.

This isn't really "pinning" a pull request. The problem is not that "GitHub Pull Requests are dynamic".

Any branch name is just an alias for the latest commit in that branch. For example master is just an alias for the latest commit in master so you can reference that commit without the SHA1 hash. When I submit a pull request for foo/master then push new commits for foo/master, I just changed what master points to so my pull request changes. By using the commit hash you avoid the commit alias problem because new commits won't change what a commit hash references.

In fact, virtually anywhere you use a branch name in git or github you can also use a commit hash.

Sure, but I'm a git luser—and I'm sure I'm in the same boat as a lot of people in thinking of 'branches' as a named branch of code in the graph, not as a kind of floating pointer to a commit.

I never presented this as a problem. But clearly the nature of GitHub's pull request UI is sufficiently misleading (or the documentation sufficiently incomplete) to wrongly convince some smart people that pull requests can always change while you're reviewing them, and that there's nothing you can do about that. In this case, that misunderstanding caused them to ignore the entire feature. I just wanted to point out a neat trick that (probably) not many others had spotted before.

Now, in actual fact I would personally choose to use standard branch-based pull requests anyway, because you get to rebase your branch while work progresses on master. But then again I've never worked on an open-source project with so many interested parties, contributors or even regular committers.

Thanks for sharing. I didn't realize you can do that with Github before reading your post.
No, but that's misunderstanding the problem. The 1:1 relationship of a branch HEAD to a pull request is an implementation detail (that happens to be exploited here).

The problem is that github thought it was a good idea to "review" such an object at all. It's the wrong data structure for that. What you want to review is a series of patches, which is also a data structure git supports, but not (quite) 1:1 with a branch head.

Why wouldn't they just comment on the pull request to set a history point and review from there? This allows changes while still being able to see what may have changed and why. Honestly I can't see how slinging patches by email works better.