Hi Peff, On Tue, 12 Nov 2019, Jeff King wrote: > On Tue, Nov 12, 2019 at 08:11:06PM +0100, Johannes Schindelin wrote: > > > > To that end, I wonder if we should add an Action to automatically > > > close PRs on that repo. It looks like > > > https://github.com/dessant/repo-lockdown would do the trick. We could > > > close incoming PRs automatically with a kind, maybe more succinct or > > > prescriptive version of the prefill text encouraging folks to open the > > > exact same PR against gitgitgadget/git instead. > > > > I am rather certain that that would not be a good thing to do. > > > > There are some people who open git/git PRs solely for the PR builds, > > others to facilitate code review, and yet others just because it is the > > intuitively obvious way to contribute to Git. > > 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: # The permission problem GitGitGadget needs code write permission on https://github.com/gitgitgadget/git so that it can push those tags that correspond to delivered patch series iterations. It also needs permission to write to Pull Requests (so that it can comment and add labels). But on https://github.com/git/git, Junio offered a strong preference for restricting access so that GitGitGadget cannot just push code. I do agree with this, but there is the complication that we cannot ask for a different permission sets depending on which repository we install the GitHub App. I just verified that I cannot add a PR comment on git/git using the existing App (which is installed only on gitgitgadget/git). Possible workaround: I could register a second GitGitGadget app (e.g. gitgitgadget2) and install that on git/git, then use that set of permissions to interact with PRs on git/git. This, however, will require a change in GitGitGadget's code, as it now potentially needs to use either the GitGitGadget App's token or the GitGitGadget2's. And for pushing the tags it always needs to use the GitGitGadget's token. BTW I do not like the name `gitgitgadget2` very much (it suggests an upgraded version to me), if you have any ideas, I'm all ears. # Disentangling the tagging part from the rest As I said, GitGitGadget pushes tags to gitgitgadget/git that correspond to each sent iteration. This is not only to allow for fetching directly (rather than trying to find an appropriate base commit and then applying the patches manually, which I find very tedious) but also for the range-diff for v2 and later. I would like to keep doing this even when letting GitGitGadget handle git/git's PRs. To avoid clashes, I would suggest to invent a new tag format. The current one is `pr-<number>/<author>/<branch-name>-v<iteration>`. Instead of the prefix `pr-`, we could easily use `git-` and be fine. However, that requires changes in GitGitGadget: so far, the URL prefix `https://github.com/gitgitgadget/git/` is pretty hard-coded. In theory, it would matter only when fetching the commits that need to be `/submit`ed, of course, but that will take some careful analysis right there. # The Checks To have the same nice integration with the GitHub Checks, where you can easily see when GitGitGadget is running, and get to the logs, we will need to install a separate CI/PR pipeline. For technical reasons, this will have to live in https://dev.azure.com/gitgitgadget/git/_build, as I want to have only one available agent for these runs: GitGitGadget is not _really_ able to run concurrently. Neither does it have to. It's not like contributors try to send multiple patch series in parallel. This saves me a lot of headache about locking. # The commit mappings One of the things that irks me the most with the mailing list driven development is that it is super hard to go from mail to commit, or for that matter, from commit to commit: _my_ commit in _my_ PR will have a completely different hash than Junio's commit in gitster/git. To help with that, GitGitGadget adds "Checks" to the commits in the PR that link to the corresponding ones in gitster/git. This should still be possible even in git/git, I think, provided that the second App would be equipped with permissions to write those checks. # The PR labels Whenever `pu` is updated, GitGitGadget tries to figure out what has been merged where, and add labels `pu`, `next`, `master` or `maint` to the PRs, closing the ones that made it to `master`. This should be equally possible on git/git, again contingent on the appropriate permission. # Reacting to `/submit`, `/preview`, etc We can probably reuse the same Azure Function that we have right now, provided that GitHub Apps can share the same webhook URL with other Apps. That's just the stuff off the top of my head. To start with this project, if I had the time, I would probably register that second app, install it on my fork and then pretend that my fork is git/git, and start testing what goes wrong (trying to re-route the mails away from the Git mailing list, of course). Not an easy, nor a small project, I am afraid. Ciao, Dscho
