git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: GitGitGadget on git/git, was Re: Should we auto-close PRs on git/git?

From
Jeff King <peff@peff.net>
Date
Nov 21, 2019, 10:54 UTC
Message-ID
<20191121105414.GA16238@sigill.intra.peff.net>
In-Reply-To
<nycvar.QRO.7.76.6.1911181930290.46@tvgsbejvaqbjf.bet>
On Mon, Nov 18, 2019 at 07:37:57PM +0100, Johannes Schindelin wrote:
Show 6 quoted lines
> 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.

Wow, thanks for working on this! I don't know that I'd call my javascript skills even rudimentary, but I did give it a look. The real challenge to me is not the individual lines of code, but understanding how the Azure Pipelines and GitHub App systems fit together. So I didn't see anything wrong, but I also know very little about those systems.

Likewise, the explanations in your comments and commit messages all made sense to me. But that may also be a false sense of security. You nicely led me through reading the patches, but the likely bug would probably be one you did not even anticipate. ;)

Show 7 quoted lines
> 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.

Yeah, that makes sense to me. Going from one repo to three is not much worse than going to two, so it's good to have a testing area, too.

Do you want any third-party testing there (e.g., a user who isn't you making a PR against dscho/git)?

Show 5 quoted lines
> 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.

I'd say "yes". The status quo is probably worse than a system with a few bugs. The worst case if it's disastrously wasting submitter's time is that we turn it back off, but I have faith that you'd just fix the bugs before then anyway.

Is the existing Pipelines integration enough for you to turn it on for git/git, or do I need to tweak any settings?

-Peff
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 10 of 22 in “Should we auto-close PRs on git/git?”
  1. Emily ShafferNov 9, 2019
  2. Junio C HamanoNov 9, 2019
  3. Stephen SmithNov 13, 2019
  4. Johannes SchindelinNov 12, 2019
  5. Jeff KingNov 13, 2019
  6. Johannes SchindelinNov 13, 2019
  7. Jeff KingNov 14, 2019
  8. Johannes SchindelinNov 14, 2019
  9. GitGitGadget on git/git, was Re: Should we auto-close PRs on git/git?Johannes Schindelin, Nov 18, 2019
  10. Jeff KingNov 21, 2019
  11. Johannes SchindelinNov 22, 2019
  12. Johannes SchindelinNov 22, 2019
  13. Jeff KingNov 25, 2019
  14. Johannes SchindelinNov 26, 2019
  15. Eric WongNov 26, 2019
  16. Johannes SchindelinNov 26, 2019
  17. Eric WongNov 26, 2019
  18. Johannes SchindelinNov 26, 2019
  19. Eric WongNov 26, 2019
  20. Junio C HamanoNov 27, 2019
  21. Eric WongNov 27, 2019
  22. Emily ShafferNov 13, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.