[HN Gopher] Git-PR: patch requests over SSH
___________________________________________________________________
Git-PR: patch requests over SSH
Author : steventhedev
Score : 138 points
Date : 2024-07-14 07:53 UTC (15 hours ago)
(HTM) web link (pr.pico.sh)
(TXT) w3m dump (pr.pico.sh)
| lifeisstillgood wrote:
| I 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 wrote:
| I 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 wrote:
| GitHub'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 wrote:
| Heh, 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 wrote:
| Grepping 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
| rkta wrote:
| After 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 wrote:
| Thank 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 wrote:
| wow, ty, this is brilliant!
| bruh2 wrote:
| This 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 wrote:
| The details of a given patch give some info about the workflow
| IshKebab wrote:
| > 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.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 wrote:
| I 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 wrote:
| > 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 wrote:
| Patch review flow was one of the thing I've really enjoyed with
| Phabricator.
| qudat wrote:
| pico team member here, I'd love to learn more about what your
| liked about using Phabricator.
| IshKebab wrote:
| Yeah 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.)
| cess11 wrote:
| Not 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 wrote:
| We are thinking about basic issue tracking, still in the "idea"
| status.
| Daviey wrote:
| Can 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 wrote:
| Sure. 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 wrote:
| This 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 wrote:
| If 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
|
| It does seem like it would be hard to support inline
| comments properly outside of an email quoting context
| without editor integration.
| j5155 wrote:
| Some IDEs actually have GitHub PR integration (JetBrains
| ones) with full comment support. It is amazing and one of
| my favorite features by far.
| satvikpendem wrote:
| VSCode has it via their GitHub extension.
| makeitdouble wrote:
| On the general idea, I'm thoroughly in support of alternative
| interfaces and workflows, so I see git-pr as a net win.
|
| 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.
| illegalmemory wrote:
| That you, looks interesting. But the license is not specified on
| Github.
| quectophoton wrote:
| GitHub'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 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 wrote:
| Although 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 wrote:
| GitHub's greatest win has been good UX. They didn't need to
| convince people much.
| xigoi wrote:
| You mean the UX where you have to make an entire fork of a
| project in order to add one line of code?
| pindab0ter wrote:
| What would the alternative be, as you can't just edit
| someone else's repo?
|
| Besides just cloning it and making the change locally, of
| course.
| aftbit wrote:
| IMO the alternative would be for GitHub to have a
| lightweight process for creating a PR that doesn't
| require me to copy the project into my namespace.
| xigoi wrote:
| Sending a patch to the maintainer.
| MaxBarraclough wrote:
| Right. Email can be used for this. This is how git was
| originally meant to be used, and this model is still used
| by the Linux kernel.
|
| See also the SourceHut forge software, which is built
| around this model.
|
| * https://git-send-email.io/
|
| * https://drewdevault.com/2022/07/25/Code-review-with-
| aerc.htm...
| Sesse__ wrote:
| > What would the alternative be, as you can't just edit
| someone else's repo?
|
| 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.
| unshavedyak wrote:
| > 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).
|
| 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?
| saurik wrote:
| The big difference is how it is organized to the users
| viewing it. My GitHub account is littered with old forks
| of repositories I created just to submit one line
| patches; and despite the pull request to a repository in
| some sense being data relating to the history of that
| project -- certainly the discussion is all organized
| under that project -- if the original person actually
| removes fork to garbage collect their namespace the
| commits referenced in that pull request just disappear.
| Meanwhile, despite people now using the word "fork" for
| this purpose due to GitHub, there is actual value in
| being able to search for _actual forks_ of a project--
| things that people are choosing to publicly distribute
| and maybe maintain themselves--rather than seeing a
| thousand repositories which exist only for the purpose of
| contributing a single patch (or, though this is another
| topic, people making the metaphorical equivalent of a
| "backup copy" within the strange set of semantics and
| ownership that is GitHub).
| jagged-chisel wrote:
| > ... git push directly to the repository
|
| Then we have a security problem or two.
| commodoreboxer wrote:
| You cut off the important part:
|
| > 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/
| mlhpdx wrote:
| It's still a security problem. If you put an unlocked
| outside door on your house and rely on the interior doors
| to be locked, we'd agree that's not safe, right? Or to
| keep it safe would require the kind of uniform
| attentiveness that people are generally bad at.
|
| Folks with accounts "littered with old forks created to
| make PRs" may not have that kind of attentiveness.
| commodoreboxer wrote:
| I don't understand the analogy. What's the actual risk
| exposed by pushing to a magic branch path that opens a PR
| instead of actually creating a branch, compared to
| creating a fork and making a PR in that way?
| quectophoton wrote:
| > 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).
|
| 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...
| PhilipRoman wrote:
| >And that `git push` doesn't need to be literally to the
| one and only repository
|
| 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.
| rurban wrote:
| This entire github fork is still cheaper than a forked
| process on windows.
| xigoi wrote:
| But then when you find an unmaintained project and want
| to find out if it has a maintained fork, you have to
| browse through hundreds of forks that were created just
| to submit one patch.
| mdaniel wrote:
| > hundreds of forks that were created just to submit one
| patch.
|
| 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
| ajross wrote:
| It's a shared/copy-on-write backend. An "entire fork"
| consists of 20-byte commit ID and whatever metadata
| identifies the owner and organization, and you create the
| "entire fork" in the UX by clicking on a single button. I
| don't understand the criticism here, do you just not like
| the use of the word "fork" because it implies a copy that
| doesn't exist?
| xigoi wrote:
| I don't like that it appears in the list of forks, making
| it hard to find actual forks.
| tiffanyh wrote:
| > "convincing everyone that 'pull request' means having a web
| interface"
|
| 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.
| koolala wrote:
| Unless... you use the Web to ssh!
| quectophoton wrote:
| Yeah that part was mostly because people tend to think in
| terms of "pull request, therefore web" or "email, therefore
| messing around with weird configs and stuff". Tell someone
| that they can use Discord instead of email and they're
| probably like "you can do that?!".
|
| 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.
| g-b-r wrote:
| Interesting, unfortunately format-patch does not support merge
| commits though.
| elric wrote:
| Would 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.
| notnmeyer wrote:
| i really am a fan of what the pico.sh folks are doing. their ssh,
| cli-centric tools really speak to me.
| qudat wrote:
| thanks!
| aseipp wrote:
| This 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/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.
| qudat wrote:
| Pico creator here. Thanks so much for your perspective, it's
| actually kind of hard to find users that live outside the pull
| request workflow. I think you are absolutely right about
| scaling issues. We have been thinking about range-diff and how
| it could work in our PR format. I'm going to read your thorough
| gist and ponder on what we can learn from it to improve our
| project.
|
| Thanks!
| djha-skin wrote:
| I 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#L...
|
| 2: https://github.com/mricon/b4
|
| 3: https://git.sr.ht/~skin/git-receive-mail/
|
| 4: https://git-send-email.io/
| tristan957 wrote:
| Why would a collaborator have to learn your email client?
| djha-skin wrote:
| They 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.
| worthless-trash wrote:
| If you just copy paste into most email clients they somehow
| screw up the patch.
| hakre wrote:
| Messaging is very simple, but made hard so that the benefit
| is not on your side. Business opportunities, yay!
| ericyd wrote:
| > 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 wrote:
| GitHub 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 wrote:
| > 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 wrote:
| Absolutely, 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 wrote:
| meanwhile 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 wrote:
| Nifty, 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 wrote:
| Github also squashes the commits of a PR before it
| merges/rebases it onto a branch though.
| maxicarlos08 wrote:
| Not necessarily, there are different ways of merging a PR in
| github
| qudat wrote:
| Not 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 wrote:
| There'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 wrote:
| > 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.
| e3bc54b2 wrote:
| Gerrit is the gold standard of code reviews and I'll die on
| that hill. I've heard many a good things about phabricator
| too, but it requires lot of special tooling while gerrit just
| uses plain old git semantics.
|
| 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).
| germandiago wrote:
| Gerrit is really good at code reviews. It is a matter of
| getting used to at first, but later it is very easy to track
| even differences between patchsets. It also marks you what
| you did _not_ reivew if a new patchset is pushed, without
| marking the files that did not change. It lets you pick the
| changes locally if you want as well. I think it is very good
| for reviews.
| Timshel wrote:
| > 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.
|
| I think it mesh well with worktree, you can setup one dedicated
| to review.
| tekknolagi wrote:
| I 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 wrote:
| SSH authentication and authorization. Everything is done via
| pubkeys. We have an ACL system for interacting with PRs and
| plan to expand it.
| zeotroph wrote:
| How 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 wrote:
| Right 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 wrote:
| Good 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 wrote:
| Gerrit 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 wrote:
| Any 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 wrote:
| Eh, (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 wrote:
| > every email through "git send-email" contains my original
| IP address for everyone to see
|
| 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.
| redleader55 wrote:
| How 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.
___________________________________________________________________
(page generated 2024-07-14 23:00 UTC)