Thread (18 messages) 18 messages, 4 authors, 2019-11-26

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help