Subject: Re: Commitfest app release on Feb 17 with many improvements

From: Jelte Fennema-Nio <[email-masked]>
Date: Fri, 7 Mar 2025 00:48:36 +0100

On Thu, 6 Mar 2025 at 18:39, Jelte Fennema-Nio <[email-masked]> wrote:
>
> On Thu, 6 Mar 2025 at 18:10, Tom Lane <[email-masked]> wrote:
> > Please see if you can make it use patch(1).  IME git is too
> > stiff-necked about slightly stale patches no matter which
> > subcommand you use.
>
> It was using patch(1) in the past for that reason, but with --3way I
> was able to get "git apply" to apply patches that patch(1) was unable
> to apply. Also with the git command you get much more useful conflict
> output.

Okay, I went for the approach of just trying everything until one
works. Starting with "git am", then patch(1), and as a final attempt
"git apply". Patch 5272 applies correctly now. I've removed any
backoff caused by repeated failures for all existing patches, so all
should be retried in the next ~48 hours.

---

From: Tom Lane <[email-masked]>
Date: Thu, 06 Mar 2025 22:08:28 -0500

Jelte Fennema-Nio <[email-masked]> writes:
> "git apply".

+1, thanks!

			regards, tom lane

---

From: Andreas Karlsson <[email-masked]>
Date: Fri, 7 Mar 2025 11:26:29 +0100

> should be retried in the next ~48 hours.

Out of curiosity: do you track which method works? I would expect 
everything to be applied with either git am or patch which can be 
applied with git apply making git apply technically unnecessary.

Andreas

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Fri, 7 Mar 2025 11:49:23 +0100

> applied with git apply making git apply technically unnecessary.

I think we need all of them...
- git apply is unable to apply the patch in 5272 on top of main
- git am, works with that 5272, but cannot apply any patch not created
by git format-patch (for instance 5522)
- I've been unable to get patch(1) not to return a failure if the
patch is already applied. So applying e.g. 5522 twice. git apply
handles that fine.

---

From: Tom Lane <[email-masked]>
Date: Fri, 07 Mar 2025 11:42:35 -0500

> patch is already applied. So applying e.g. 5522 twice. git apply

Hm, don't you *want* a failure if the patch is already applied?

			regards, tom lane

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Fri, 7 Mar 2025 18:52:38 +0100

> Hm, don't you *want* a failure if the patch is already applied?

It's pretty common that in a larger patchset the first 1-2
small/trivial patches get committed before the rest. Having to then
send an additional email, resubmitting the rest of the patchset
because CFBot fails when re-applying those is kind of silly. This
avoids that.

---

From: Tom Lane <[email-masked]>
Date: Fri, 07 Mar 2025 12:57:25 -0500

> because CFBot fails when re-applying those is kind of silly. This

Got it.  That is indeed an improvement, though of course it only
helps if those patches were applied verbatim.

			regards, tom lane

---

From: vignesh C <[email-masked]>
Date: Sun, 9 Mar 2025 07:50:54 +0530

On Fri, 31 Jan 2025 at 19:54, Jelte Fennema-Nio <[email-masked]> wrote:
>
> At the FOSDEM dev meeting we discussed potential improvements to the
> commitfest app and how to handle deploying future changes with minimal
> disruption to existing workflows. We're going to try a new approach:
> announcing a new commitfest release ~2 weeks in advance, including
> release notes. That way people can try out the changes on the staging
> environment and raise concerns/objections. So hereby such an
> announcement:
>
> The next commitfest app release will take place on February 17 and
> will contain the following user facing changes:
>
> 1. Showing CFBot status on patch and commitfest pages

Couple of suggestions: a) No need to show CI status as "Needs rebase,"
"Not processed," etc., for committed patches. b) Can we add a filter
for "Needs rebase!"? This would help the CommitFest manager easily
list patches that need updating.

Regards,
Vignesh

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Sun, 9 Mar 2025 14:35:34 +0100

On Sun, 9 Mar 2025 at 03:21, vignesh C <[email-masked]> wrote:
> "Not processed," etc., for committed patches.

Do you mean specifically for committed ones? Or just for any patch
with a "closed" status.

> b) Can we add a filter

That should be pretty easy to implement. But is that really what we
want? In the next release, sorting by "failing since" is implemented.
It sounds like that could be enough instead. i.e. do we really only
want to call out patches that need a rebase? Or also ones that have
been failing in CI for a long time?

I'm even wondering if this whole flow still makes sense. Do we really
want to send an email to  the mailing list about this? And if so, why
is someone doing that manually? If people subscribe to updates for
patches that they authored, then they get these "needs rebase"
automatically. Should we maybe simply default that option to true? And
for instance send a notification automatically to all people with a
"needs rebase" CI status whenever we start a new commitfest.

---

From: vignesh C <[email-masked]>
Date: Tue, 11 Mar 2025 09:17:16 +0530

> Do you mean specifically for committed ones? Or just for any patch

It is for any closed status patches.

> "needs rebase" CI status whenever we start a new commitfest.

It will be good if you can send a notification automatically to the
patch requiring a rebase when the CFBot first identifies this(not only
during the start of commitfest) and probably send this notification
once in a day for 3 or so days and then change the status to "waiting
on author" if a new rebased version is not posted in this time.

Regards,
Vignesh