GitGitGadget on git/git, was Re: Should we auto-close PRs on git/git?
From: Johannes Schindelin <hidden>
Date: 2019-11-18 18:38:28
Hi Peff, On Fri, 15 Nov 2019, Johannes Schindelin wrote:
On Thu, 14 Nov 2019, Jeff King wrote:quoted
On Wed, Nov 13, 2019 at 01:04:35PM +0100, Johannes Schindelin wrote:quoted
quoted
We talked a while ago about having GitGitGadget operate on git/git, rather than on a separate mirror. That would automatically help at least one class of PR-opener: people who want their patches to reach the list but didn't realize they should be using gitgitgadget/git. I don't remember what the technical blockers are for getting that set up, but it seems like a strictly nicer outcome than auto-closing their PR.Okay, here are a couple of technical challenges, off the top of my head: [...] Not an easy, nor a small project, I am afraid.Yow. That's a lot more involved than I was hoping for.
Yeah, it wasn't easy. But then, who does not like a little challenge, especially the challenge to test things outside of production? So here is a PR: https://github.com/gitgitgadget/gitgitgadget/pull/148 I trust everybody with even rudimentary Javascript skills to be able to provide useful feedback on that PR. To build some confidence in my patches (as you probably know, I do not trust reviews as much as I trust real-life testing, although I do prefer to have both) I "kind of" activated it on my fork, limited to act only on comments _I_ made on PRs (and sending only to me instead of the list), and it seems to work all right, so far. I cannot say for sure whether it handles the PR labels correctly, but I guess time will tell, and I will fix bugs as quickly as I can. Question is: should I turn this thing on? I.e. install that GitGitGadget-Git App on https://github.com/git/git? This would allow GitHub users to `/submit` directly from PRs opened in that repository. I am sure that there are a few kinks to work out, but I do think that it should not take long to stabilize.
quoted
Thanks for writing it up. Some of the points raised were interesting. I do think we'd want git/git (the repository) to remain read-only if possible.I guess you're right. We should probably try to restrict the permissions as much as possible, not only deny write access to the repository. For example, one thing GitGitGadget does is to add these "Checks" to the commits of the PRs which contain links to the corresponding commits in gitster/git (if any). Those can actually not be removed, there is not even any API for that. So it would probably make sense to avoid that in git/git. This would mean that the git/git part of GitGitGadget does not install those commit mappings. I guess that's okay, they _are_ kinda hard to use.
I made it so. The GitGitGadget-Git App only requires write permission to add PR comments and labels, which I think should be okay. It specifically has _no_ permission to push to git/git.
quoted
If GitHub's permissions model is a limiting factor here, let me know and I can try to bring it to the attention of the right people.I actually don't think that my use case fits any sane permission model ;-) After all, I want the GitHub App to _span_ repositories (even orgs), and that's not really the idea of Apps. After sleeping over it, I don't actually think that it is such a bad idea to add a second GitHub App with a more limited permission set.
The name _was_ bad, but I did settle for GitGitGadget-Git in the end. Not the most elegant name, but hey, it works so far. Ciao, Dscho