Git-PR: patch requests over SSH
pr.pico.sh
pr.pico.sh
Keeping the user in their editor for code reviews is pretty nice indeed, but I think that with larger projects and bigger changes, it's going to be way more difficult to skim through what has changed where, which helps me identify how to start reviewing. There would have to be an easier way to see what changed where quickly without having to mess up my working tree (which is almost always dirty with something) and having to stash or switch back and forth between branches for reviewing is going to be painful. Sometimes I have small comments or questions, I'd like to be able to do that without having to download the entire patch.
Also, I'm not sure if I missed something, but does every review get applied as a commit by itself that I (as repo owner) would have to squash? This feels like a chore by itself.
I have relied on github for previous work but for the last couple of years I've been primarily using Gerrit for my job, and although there are some drawbacks, I absolutely love its workflow. Github workflow is not free from drawbacks either, and if I'm asked now I absolutely prefer Gerrit's to Github's. Gerrit uses Git references in a unique way to facilitate code reviews and patches (which consist of one or more patchsets). I rarely use branches now except for extremely long-lived experiments, or for local temp purposes. A patch gets merged as a single commit no matter how many rounds of patchsets the submitter adds and reviews they get, and when needed I can quickly download the patch at any patchset to a different branch and then delete it.
I think it mesh well with worktree, you can setup one dedicated to review.
I love this feedback and totally agree with you. Pulling a PR down and applying to review is great until you need to reference what's in main and have to ping-pong between them. At the end of the day, `git-pr` supports pushing and pull patchsets and we do have ideas around how a reviewer could review a patch request without applying it locally for something lighter-weight. Something like editing a patchset directly (with comments) and uploading that.
I think I need to play around with Gerrit. When originally developing this idea, I looked at Gerrit but didn't actually play around with it.
Someone else mentioned this already but I do want to experiment with revisions and supporting `git range-diff` workflows which I think could provide better support for larger reviews.
That said, I live git-pr so far. I have a forgejo instance and I like it for what it provides, but without federation, external contributors aren't able to contribute.
If git-pr can be made to work with forgejo, I'll just go for it (self hosted, of course).
The downsides of email workflow that the author mentioned are (1) Having to setup mailing list, (2) setting the mail client, (3) friction in submitting a patch, which I assume the author is referring to setting git-send, (4) email is limited such as not being able to modify it, not being able to download the patch, and limitation around the plain-text, and the author doesn't hint into what limitation he is thinking of.
These are downsides of email workflow, and I do not think they are significant (except for 1, which could be done though a person's email address if the project is small). But the SSH workflow replaces them with similar issues such as (1) setting SSH server and maintaining it and the cost associated with it, (2) clients will need to maintain their identities though SSH, (4) replace email-replies with code comments (email-replies is better as you can do more than just text). (3) Here SSH workflow is better as it requires no work, but configuring git-send is a very small and usually 1-time change.
In most cases, the only mail client requirement is to be able to write plain text. Virtually all mail clients can do this with a simple checkbox. Only the maintainer will need ability to `git am' the patch from an email message, and this is indeed a downside of email workflow.
All in all, nothing wrong with more options, and I am sure SSH meets certain needs, but to me email-workflow and SSH compete in the same category and email-workflow is superior.
Even now there are persistent downsides: every email through "git send-email" contains my original IP address for everyone to see, unless I use a VPN service. Also, since I'm not a right-thinking person with a 'real' client-based inbox, I have to fiddle with the In-Reply-To and References headers by hand, if I want to include a patch in a reply to another email.
Is this the fault of git or your SMTP provider? I looked through a mailman archive and couldn't find my client IP address. The first Received headers refer to my email host.
- have an account on the platform
- fork the repo
- pull repo
- create a branch
- make the change
- push to forked repo
- click a button in the UI to create the PR
With `git-pr` it can be this simple:
- pull repo
- make the change
- submit pr with a single command `git format-patch | ssh host pr create x`
If you've ever used the email workflow I don't think I need to explain why it sucks.
> If you've ever used the email workflow I don't think I need to explain why it sucks.
I prefer it FWIW. After setting up your Email, it is much easier and faster to send a mail than go through a PR-process. On the reciving end you can respond inline to the patch, in the compfort of your email client, without having to use a browser.
My feeling is that people who don't like this, don't have a good mail setup (which is understandable given some hosts), making Email much more painful to use in general.
It does seem like it would be hard to support inline comments properly outside of an email quoting context without editor integration.
On the above steps, I feel it's misrepresenting what github/gitlab have to offer. The only real step would be "have an account on the platform" and looking at the file in a browser. From there creating a MR/PR in a new branch with the changes you want is painless and a matter of 3 or 4 clicks.
It works very well for repos with config files that only need the CI to be green, you get a full MR in 1 min if you want to.
Notice that `git-pr` still has a centralized service, just an SSH-based one rather than HTTP-based. I don't see a win there.
> 4. External contributor submits a PR to SSH server But how? That numbered list let me think of that old cartman profit meme.
I like the idea, but I'm not convinced yet, that this is really easier than just sending an email by `git request-pull`.
Phabricator was pretty nice overall I would say. The bug tracker in particular was good. I have to use Jira now and it's night and day.
The big flaw was there was essentially no CI support. That and the fact it was written in PHP, which you'd think wouldn't matter, but I did end up being forced to learn some PHP to fix Phabricator bugs and write custom linters.
Now we're using Gitlab which is also decent except it's written in Ruby which if anything is worse than PHP! Absolutely unreadable code base. I can't fix anything. (Except in gitlab-runner which is written in Go - I've contributed several fixes & features there.)
"just" - I bounced off Phabricator because I couldn't use native git tooling, and what mapped to where wasn't clear to me. The pure git approach of git-pr is the exact opposite of that. Sure, someone is going to write just-too-useful-to-skip-wrappers around that later, but you always know what's going on under the hood.
I do dream of one day tackling a whole load of these developer “gold plated porcelain” problems - everything from a “glimpse” index of every commit in a repo to better Jenkins config and how to set up mTLS the hard way.
It’s “Thedevmanual” and someday I will have time to write it
Definitely not my experience with VSCode and GitHub's PR extension, but I don't think I'm the target audience for this product anyway.
What about other editors and IDEs? It might work great for VSCode, heavily leveraging vertical integration, but everything else is out of scope.
`git-pr` removes all of this overhead by being creative with its design. You don't need a VSCode extension, just write the review in code and we help make that manageable.
What like this? https://marketplace.visualstudio.com/items?itemName=GitHub.v...
I'm all for standardising the pull request process but their suggested workflow sounds way worse than what we have now. Given how dominant Github is I seriously doubt anyone except the nerdiest nerds will use this.
IMO if anyone is going to solve this it's going to have to happen inside Git, which I can't see happening.
Our target demographic is the self-hosted hacker enthusiast. If you want a full-blown pull request workflow in the browser, this will never be that.
However, there's a large demo of users that want a simple self-hosted solution that doesn't require a bunch of infra to manage. If you want to self-host a git collaboration tool you basically need to bootup gitea-scale services or be relegated to `git send-email`. The sweet spot for `git-pr` is right in-between those two tools.
All you need is a tiny VM and a golang binary and you have a code review tool that is leveraging utilities you already have installed: git, ssh, and an editor. End users do not need to create an account, install any clients locally, and all user personas and workflows can be accomplished in the browser.
Is that actually difficult though? I was always under the impression that Gogs/Gitea were pretty easy to run because they're also written in Go.
I just checked Gogs and it looks like it has exactly the same dependencies as Git-PR (Git and and SSH server).
It would definitely be cool if Gogs/Gitea/Gitlab/Sourcehut standardised a PR API, but... like properly.
1) push 3 commits (author)
2) push 3 commits + 1 review commit (reviewer)
or even 2a), + 3 interleaved review commits, might be needed if the final refactor removes something.
then
3a) force-push 3 commits with the review comments applied (you mentioned --force is supported)
3b) or push 3 + 1 + (worst case 3) fixup commits, then a squash later?
If 3a), does pico have the concept of patchsets like gerrit, so the state 2) can be retrieved later?
PS: ... or even 2c, if the line I am commenting on is removed in patch 2 I would need to push a whole "review tree" (or fix conflicts in b and c):
a -- b -- c -- review-c
\ \
\ `-review-b
`- review-aWe do see this as a problem and are thinking about ways to address it. We do support the idea of a patchset (that's what the patch request managed). Right now we only support a single patchset for a patch request, but we are leaning toward introducing patch revisions where both contrib and owner submit entire versions of the patchset. This could provide better tracability since reviews would be in a separate patchset "channel" and not lost to force pushing.
We also plan on supporting a patch request "cover letter" which includes all the events and comments during review. This cover letter is just an empty patch which could be `git am --empty=keep` or `git am --empty=drop` depending on preference.
And even if everything git-pr does could be done by talking to the Gerrit REST-API, a simple straight forward and lightweight implementation of the "essence" of this model is very useful.
By the way, good luck getting that set up to work on Windows, which a large population of developers use because of their work laptops (or because they simply prefer the interface!).
I made a script[3] that makes it easy to accept patches using an fzf interface, as a sort of reply to git-send-email[4]. I made sure to get it to work on Windows as well.
I kind of think I might like this better. My script only involves some setup of two other third-party tools to accept a patch, but this feels a little cleaner.
1: https://git.sr.ht/~skin/dotfiles/tree/main/item/dot-muttrc#L...
2: https://github.com/mricon/b4
If you are only accepting 1 patch a year, then the reality is anything will work. Email, uploading .patch files to a bug tracker, GitHub, whatever. But if you're doing review and managing a dozen active patch sets at once with people who are asynchronous to you, then you need something better than that. Let's say 5-10 active contributors. That's where projects actually tend to struggle.
There's this book called "Rethinking UI" and one of the first things it talks about is to write down features for your system, because the user interface comes from features. You can't design the UI without laying out a list of features. What features do I need in a code review tool? The two most important ones IMO are:
1. Code review is iterative, so a good feature is being able to see the history of a change as it is iterated on.
2. It's normal to have many patches in flight (your own + others). So, it's very useful to be able to quickly find things in a particular state: you need to review it (patch from someone else), you need to update it (patch you wrote and that got reviewed), it needs to be merged.
Extra:
3. It should be easy to safely integrate a change when it's ready.
Most tools are very bad at these core ideas:
- Almost no actual mainstream review tools, or modern pop-up clones, ever include "git range-diff" in their repertoire of tools. range-diff is essential for many projects to do iterative review, and tools like Linux's b4 automate them. I won't include it all here, but I wrote about this review style elsewhere: https://gist.github.com/thoughtpolice/9c45287550a56b2047c631...
- Most tools fall apart at "What state is this in." For instance, in GitHub it's impossible to tell what state anything is in, except "Can be merged" or "Cannot be merged." For instance, do I need to do another review on this PR after I did one before, and a coworker did one too? I have to go and look to see if everything was addressed properly. Tools like Gerrit have a more granular concept called "The Attention Set" which formalizes the concept of "Do I need to do something?" because it shows you when you need to take an action, like re-review or merge a patch.
git-pr doesn't really seem to make those core problems any easier.
Your approach sort of sidesteps these, these because you're expected to apply code manually and view it in your own editor. You could range-diff on your own. Part of the argument here goes back to the argument that best thing to do is sit in your editor -- but with a good review tool, you can do lots of review purely from your tool with no editor involved! I did tons of code review with Gerrit every day without ever opening my editor for 99% of it.
A semi-related thing some people say is "Well, in the Linux kernel style you can just send a pull request through any medium", while GitHub and these other tools use a specific interface. The problem is you still need state to track the state of the work. That's required (it's one of my features!), notably so you can look at the change evolution and see if it's ready to merge. If I tell you "Please merge my branch from https://example.com/foo.git with tag xyz-123" that's technically a "pull request" like you would send to Linus, but understanding the actual state that PR is in is impossible without a lot of extra knowledge that a third party tool formalizes (even if poorly.) b4, the Linux kernel patch series tool, has a ton of stuff for things like this like digging up the right email threads to do diffs between, showing the cover letter, etc.
And in the "How do Patch Requests work?" section, this sort of shows exactly the problem with this approach. I don't want to manually apply patches and run commands to merge it and push manually! I have 10 other patches to read after this one! Integrating things is why my tool is there! Why are steps 13 and 14 even necessary, for instance?
Gerrit does all 3 of the above properly and is a very, very powerful and productive tool when you get used to it.
Your approach does have a conceptual advantage which is that it focuses on the patch and not the "branch" as the unit of work. This is smart. So, if you keep going down this road and follow it to its logical conclusion, I think you'll tread some familiar ground. But I think realistically, you need to track actual state a patch is in, so that you can do things like range-diff it -- that would make the appeal of "Just write comments in the code" or whatever much higher because at least you can see when those comments get handled. You'll want to think about concepts like "Change IDs" as Gerrit calls them. Etc.
BTW, Gerrit does all if this, it uses this 'Git notes' approach you briefly mention, and it stores all its content in a Git repo too, no extra database. The web UI has tons of affordances, and everything you do in the web UI is just a commit in the underlying repo. And you can interact with Gerrit entirely over SSH with pubkeys if you want; it looks like running normal commands e.g. `ssh austin@gerrit.foo.com gerrit review ...` -- in theory you could write a whole TUI. I strongly suggest spending some time with it, if you haven't.
Thanks!
https://blog.sesse.net/blog/tech/2024-07-15-13-04_pull_reque...
The goal is to provide the flexibility of a pull request with the simplicity of generating patches.
At the end of the day, the maintainer of the repo has to load the patchset locally, apply it, fix it up, then push to upstream. They have full control here.
One of the required features of a review flow is having a way to look at a patch, understand the reasoning behind it, and the thoughts of people that reviewed and accepted it.
I found the text a bit too reference-y, as in giving too much theoretical background, while what I'm actually looking for is workflow breakdown. The demo video at the top of the page[1] demonstrates it beautifully and I recommend you go give it a watch.
They even convinced people that repository clones are called "forks" when they have a web interface but not when they're local.
I like that this project is trying to remove those misconceptions.
Of course if the contributor already has their own publicly available repository, then they can send you a pull request through HackerNews if they want.
Hey I made some performance improvements, they are at https://example.com/git-pr.git on branch `improvements`.
I've also done ad-hoc patches through Slack, using `git diff` and `git apply` but the idea is the same.
And yes I've also done stuff like this but with just `git push` instead of `git format-patch | ssh` (you want to look at pre-receive hook) messing around with friends on IRC. The problem is not how possible or easy it is, the problem is "just" gaining traction and having a good plan for moderating The Stuff People On Internet Will Upload.
See GitTorrent for another (unrelated) example of a good idea that never gained traction.
Or, worse (IMHO) when they created the fork to house a patch - sometimes a meaningful feature, bugfix, or security fix - and then left it because the submission process was too onerous to bother. Onerous can also include never getting the PR reviewed, too, which is far more damaging (IMHO) because it disincentivizes future contributions, too
I actually would love an "implied PR" view of the forks which would enable quickly filtering out the -0,+0 "shallow forks" from the -50,+2000 "oh, that's likely doing something interesting" ones, provided it has the sane ?w=0 to hide forks that felt it necessary to push up simple reformatting changes
Edit: I guess they would just not delete the data if there were more references to it.
Besides just cloning it and making the change locally, of course.
git push directly to the repository, in a separate branch namespace. This is how e.g. Gerrit works (pushing to a special ref makes a review, which is essentially the same as a pull request).
> Besides just cloning it and making the change locally, of course.
With GitHub, you cannot do that and get a PR out in the other end. You _must_ fork the repository into your own user/organization, push to that and then send a PR from that.
And that `git push` doesn't need to be literally to the one and only repository. The SSH daemon could create an isolated environment (e.g. QEMU, FreeBSD jail, etc) that contains a copy of the repository, and run the commands in there. Obviously this could also check SSH keys and the requested git commands before doing anything at all.
It would probably be like what Sourcehut does[1] for letting you SSH into build VMs, but instead of a build it's a push. And they already do some logic during a push[2], so their code for those two places is probably a good place to look for how to implement this kind of thing.
[1]: https://man.sr.ht/builds.sr.ht/build-ssh.md
[2]: https://sourcehut.org/blog/2019-11-22-what-happens-on-git-pu...
I believe github already has their own implementation of a git server, so any commands submitted to it are abstracted away. They probably don't have a literal .git directory sitting on a server.
What's the material difference? They build special mechanisms to provide access control for sub namespaces, which sound a lot like "forks".
Also i have no clue on their backend (iirc this info is researchable tho), but i wouldn't be surprised if functionally that is exactly how they do it anyway. It's all content addressed, i doubt they pay 2x the storage anytime you fork a repo right?
Then we have a security problem or two.
> in a separate branch namespace
So not really. It's a special branch path that only exists for opening PRs, and doesn't do anything other than opening a PR. Yes, they share an object space, but so do forks in the first place, so any security issues with this flow are the same ones in the fork-PR flow.
You can check out this which covers the whole flow: https://git-repo.info/en/2020/03/agit-flow-and-git-repo/
Or for a simpler overview, look at Gitea/Forgejo's implementation: https://forgejo.org/docs/latest/user/agit-support/
Folks with accounts "littered with old forks created to make PRs" may not have that kind of attentiveness.
See also the SourceHut forge software, which is built around this model.
* https://drewdevault.com/2022/07/25/Code-review-with-aerc.htm...
> the problem is "just" gaining traction and having a good plan for moderating The Stuff People On Internet Will Upload. See GitTorrent for another (unrelated) example of a good idea that never gained traction.
maybe there is a reason why github gained traction
What’s more convenient, making this accessible via the web … or forcing the use of SSH?
I don’t disagree with your point, and find pico.sh super interesting - but web is a much lower bar than ssh.
It's the same as when people say "GitHub" when they mean "Git"; or when they ask for your GitHub[1] to read some code. It's understood what they're trying to say, but thinking that "GitHub" is the same as "Git" causes unnecessary bias in thoughts.
Using a different ecosystem as example, you can see this happening with ActivityPub implementations being biased towards Twitter-like usage thanks to Mastodon (where here ActivityPub would be equivalent to Git, and Mastodon equivalent to GitHub).
But thankfully there's people who know they shouldn't be conflated, and are slowly bringing us collaboration between Git repositories through ActivityPub, starting with a web implementation (because as you say it has the lower barrier to entry). This should open more doors to more creative "bridges" that integrate with this ActivityPub flow.
A more clear bias you see everywhere is that Git forges tend to just copy GitHub's flat URL structure of "username/reponame". It's rare to find something that does not have a flat structure like that (e.g. "username/hackernews/repo-for-that-thread") (yes I'm keeping "username" in the URL on purpose but it doesn't need to be there). Don't get me wrong, I like to organize things relatively flat most of the time, but I really appreciate a nested structure when I mirror thirdparty repositories (e.g. "example.com/mirrors/github.com/username/reponame").
So yeah, I like that from time to time we're getting projects like this that try to improve git collaboration and start from a point of view different than mimicking GitHub on everything.
[1]: I've noticed when people ask for repos to read code they tend to ask for a GitHub link, but it's understood that a link to some Codeberg/Gitea/cgit/stagit instance also works.
i feel like branch permissions in bare git repos should be a thing already. (rather than, say, having to rely on third-party providers implementing it into their centralised-git web service)