10 ms·
Git-PR: patch requests over SSH
- lifeisstillgood 2y agoI like the basic philosophy that everything should be inside the code (yes lots of great collaboration and review tools out there with great browser based diffing- it’s just I am thirty years of reading code and writing code in a tool and I kinda get used to the code in the tool
- pushfoo 2y agoI agree for smaller projects, especially personal ones. For larger projects, approaches like GitHub's web client + terminal interface might be the next best thing since grepping doesn't scale well. For better or worse, GitHub branding also helps make their specific terminal tool an easier sell in large org environments.
- tristan957 2y agoGitHub's web client does not scale at all in my opinion. It is very slow to load source large patches, and the code navigation experience is not great, hence why they brought VSCode for Web to the product, but still.
- xelamonster 2y agoHeh, well GitHub's search doesn't exactly scale well either. They _still_ only index the main branch, so if you want to find anything under active development or on an old tag version you're out of luck, clone + grep is the only way.
- lifeisstillgood 2y agoGrepping for something “I am sure it was on a development branch somewhere” is always painful. 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
- krisoft 2y agoI don't really understand this argument. It is not like a web based tool is stopping you from seeing the code in your editor. You just check out the code and now you are seeing the code in your fav editor. Which is exactly the same process you would use with this tool.
- rkta 2y agoAfter reading the website and also trying the mentioned `ssh pr.pico.sh help` I still don't know what exactly to do to use this. > 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`.
- qudat 2y agoThank you for that feedback, I removed a couple important pieces from the repo readme that I think would answer your questions around usage. I'll add them into the main site.
- michael-ax 2y agowow, ty, this is brilliant!
- bruh2 2y agoThis is amazing! 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.
- rakoo 2y agoThe details of a given patch give some info about the workflow
- IshKebab 2y ago> Github is bringing the IDE to the browser in order to support their workflow, we want to flip that idea on its head by making code reviews a first-class citizen inside your local development environment. What like this? https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github 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.
- qudat 2y agoI think that's a fair observation. We don't expect massive companies to use this tool. Even within GitHub many companies are feeling its PR review feature-set lacking, especially for massive, long-standing code changes (just look at all the companies productizing code reviews). 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.
- IshKebab 2y ago> you basically need to bootup gitea-scale services 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.
- bobek 2y agoPatch review flow was one of the thing I've really enjoyed with Phabricator.
- qudat 2y agopico team member here, I'd love to learn more about what your liked about using Phabricator.
- IshKebab 2y agoYeah me too. I used Phabricator extensively and in practice it is basically the same as Gitlab and GitHub and everyone else. You just say `arc diff` instead of `git push`. 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.)
- zeotroph 2y ago> just say `arc diff` instead of `git push`. "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.
- wyldfire 2y agoI think gerrit seems to use this same kind of workflow, but with a git frontend (but just for transmitting the patches). More convenient than arcanist IMO.
- cess11 2y agoNot sure what I get from this that I won't from Gitea, and that doesn't need any adaptations to interfaces and if I want I get some basic project tracking tools and so on.
- qudat 2y agoWe are thinking about basic issue tracking, still in the "idea" status.
- Daviey 2y agoCan you expand a bit more why this workflow (send patch over ssh, and rss feeds for owner), is better than either email workflow or using a centralised service with Pull Requests.
- qudat 2y agoSure. Pull Requests tend to force users into collaborating in a web view. Further, they require a collaborator to: - 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.
- pkal 2y agoThis seems to come very close to recognising that if someone without permissions pushes something to a repository, it should be convered into a PR. > 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.
- johnny22 2y agoIf you review PRs as whole, you don't need to use the web interface. I think it only gets harder when you want to do inline comments. At least that's what the docs for github cli suggest. https://cli.github.com/manual/gh_pr_review https://cli.github.com/manual/gh_pr_review It does seem like it would be hard to support inline comments properly outside of an email quoting context without editor integration.
- 2y ago
- kiguri 2y ago[flagged]
- illegalmemory 2y agoThat you, looks interesting. But the license is not specified on Github.
- quectophoton 2y agoGitHub's greatest win has been convincing everyone that "pull request" means having a web interface for code review as well (conflating sharing code with reviewing it) and can only be done by creating accounts in websites; and that patch-based workflows can only be done through email. 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`. That was a pull request. You can check out changes with `git pull https://example.com/git-pr.git https://example.com/git-pr.git improvements` and review them. 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.
- ithinkso 2y agoAlthough I do agree with your overall sentiment > 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
- LudwigNagasena 2y agoGitHub's greatest win has been good UX. They didn’t need to convince people much.
- xigoi 2y agoYou mean the UX where you have to make an entire fork of a project in order to add one line of code?
- elric 2y agoWould be even nicer if the command could be simplified (perhaps using a git alias) so that users don't have to go through the hassle of piping through ssh by hand.
- g-b-r 2y agoInteresting, unfortunately format-patch does not support merge commits though.
- notnmeyer 2y agoi really am a fan of what the pico.sh folks are doing. their ssh, cli-centric tools really speak to me.
- qudat 2y agothanks!
- aseipp 2y agoThis looks kind of small and homely and is obviously designed to appeal to a certain niche, which is nice. But the problem with a lot of "alternative code hosting" tools like this, IMO, is that they aren't actually that good for code review. This really dulls their appeal. What's the point of setting all this up if I don't read the patches? 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/9c45287550a56b2047c6311fbadebed2 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 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.
- djha-skin 2y agoI recently switched to neomutt and offlineimap/isync as my email client. I tried really hard after that to get a setup going so that I could accept git patches mailing list wise. I had to figure out how to set up a keyboard macro with just the right incantation[1] of b4[2]. I realized accepting patches meant my friends would also have to learn (neo)mutt if I wanted them to accept mine. It was really difficult. I can't imagine asking my friends to do that and collaborating with them. (I still like mutt though and still use it as my main client.) 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#L107 https://git.sr.ht/~skin/dotfiles/tree/main/item/dot-muttrc#L... 2: https://github.com/mricon/b4 https://github.com/mricon/b4 3: https://git.sr.ht/~skin/git-receive-mail/ https://git.sr.ht/~skin/git-receive-mail/ 4: https://git-send-email.io/ https://git-send-email.io/
- tristan957 2y agoWhy would a collaborator have to learn your email client?
- djha-skin 2y agoThey would need to learn a email client. The patch is in their email inbox and they don't have an easy way of applying it to their git Repository. Aerc or Mutt is necessary to accept patches easily usually, but then you have to learn those tools.
- tristan957 2y agoMakes sense. Thanks!
- worthless-trash 2y ago
- ericyd 2y ago> There are tools and plugins that allow users to review PRs inside their IDE, but it requires a herculean effort to make it usable. 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.
- tristan957 2y agoGitHub has done a good job of getting the product into VSCode. I think this product is targeting people who already likely using a terminal editor and work well within a CLI environment, which is what all of Pico's services are trying to do already.
- qudat 2y ago> Definitely not my experience with VSCode and GitHub's PR extension 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.
- ericyd 2y agoAbsolutely, I was just responding to the herculean effort part. I realize it's a nit, but I think it might be more accurate to say that not all editors are supported, because in some cases it takes barely any effort at all.
- tpoacher 2y agomeanwhile the equivalent process in svn is simply "svn copy" followed by "set permissions". 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)
- EPWN3D 2y agoNifty, but it seems like this would lose commit message content and history entirely during the course of the review since you have to submit a single patch that gets applied, rather than publishing commits to a branch that gets merged. Am I understanding correctly?
- beepbooptheory 2y agoGithub also squashes the commits of a PR before it merges/rebases it onto a branch though.
- maxicarlos08 2y agoNot necessarily, there are different ways of merging a PR in github
- qudat 2y agoNot quite. We have adapted the `git format-patch | git send-email` flow to support editing the patchset. You can add, remove, even `--force` replace all patches within a patch request. 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.
- beshrkayali 2y agoThere's definitely something very interesting here. 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.
- qudat 2y ago> 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. 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.
- tekknolagi 2y agoI think permissions are a bit wonky? I could, as a random person, close some other project's PRs. How is the authz supposed to work?
- qudat 2y agoSSH authentication and authorization. Everything is done via pubkeys. We have an ACL system for interacting with PRs and plan to expand it.
- zeotroph 2y agoHow do you envision pushing/reviewing a branch of 3 cleanly separated commits (say: write failing test, fix code, refactor) into a repo with a fast-forward/no-merge policy? The end result should of course be 3 commits, but review history should not be lost. 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-a
- qudat 2y agoRight now, when force pushing, we preserve event logs for every server mutation, but the review themselves are lost. We 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.
- zeotroph 2y agoGood to hear, Gerrit does a lot of things right, see "stacked diffs" (or the realization that GitHub lacks these) now becoming more popular. And the patchset model is one important aspect of that, especially being able to see the difference just between patchsets - which git-pr can then do via `git range-diff` (which you already mentioned). 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.
- germandiago 2y agoGerrit allows you to see patchsets as they are applied on the same merge request. What are exactly the differences? It looks to me like similar to what you would do with Gerrit.
- gtest 2y agoAny solution has its cons. Github workflows has downsides and this solution does solve them (and introduces its own). But what I am not getting is why this approach is better than the email workflow. 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.
- LegionMammal978 2y agoEh, (2)/(3) can be a pain for web mail accounts. For instance, I couldn't send patches to LKML through Gmail directly, since even for plain-text emails it wants to rewrap long lines itself. To get "git send-email" working through it, I had to wade through a bunch of outdated info to find the right method. 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.
- dustyharddrive 2y ago
- redleader55 2y agoHow does the data backend for this look like? Does the review live after the PR being merged? 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.
- zamalek 2y agoThis is so slick. It's amazing to see how well git works when used in the way Linus intended. It seems to be missing some fundamentals (readme.md, signing, ssh key rotation), which I might try contributing to get my feet with :)
- eranation 2y agoWhat about status checks and workflows?
- Piraty 2y agoworkflows can be as slim and efficient as this: https://news.ycombinator.com/item?id=37086022 https://news.ycombinator.com/item?id=37086022
- cacozen 2y agoThis is extremely interesting. Add federation and you have a decentralized GitHub
- pabs3 2y agoThis is another interesting approach: https://blog.sesse.net/blog/tech/2024-07-15-13-04_pull_requests_via_git_push.html https://blog.sesse.net/blog/tech/2024-07-15-13-04_pull_reque...