Thread: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest

I'd like to send an automatic mail to a thread whenever it gets added
to a commitfest. Since this would impact everyone that's subscribed to
the mailinglist I'd like some feedback on this. This mail would
include:

1. A very short blurb like: "This thread was added to the commitfest
with ID 1234"
2. A link to the commitfest entry
3. A link to the cfbot CI runs
4. A link to the diff on GitHub
5. Any other suggestions?

The main reason I'm proposing this is that currently it's not trivial
to go from an email in my inbox to the commitfest entry. This can be
especially hard to do when the subject of the email is not the same as
the title of the commitfest entry. This then in turn increases the
barrier for people to change the commitfest status, to look at the CI
results, or look at the diff on GitHub. I also had once that I
accidentally added a thread twice to the commitfest, because I forgot
I had already added it a while back.

To be clear, this would **not** be a repeat email. It would be sent
only once per thread, i.e. it is sent when the thread is added to a
commitfest entry. So there won't be a flood of new emails you'll
receive.

Also the github link does not allow comments to be posted to github,
it's read-only. An example being:
https://github.com/postgresql-cfbot/postgresql/compare/cf/5025~1...cf/5025

Jelte Fennema-Nio <postgres@jeltef.nl> writes:
> I'd like to send an automatic mail to a thread whenever it gets added
> to a commitfest. Since this would impact everyone that's subscribed to
> the mailinglist I'd like some feedback on this. This mail would
> include:

> 1. A very short blurb like: "This thread was added to the commitfest
> with ID 1234"
> 2. A link to the commitfest entry
> 3. A link to the cfbot CI runs
> 4. A link to the diff on GitHub
> 5. Any other suggestions?

+1 for the basic idea.  An awful lot of people send such mails
manually already, so this'd replace that effort in a more predictable
way.  Not sure about your 3 & 4 points though, especially if that'd
mean some delay in sending the mail.  For one thing, wouldn't those
links be stale as soon as the initial patch got replaced?  Those
links seem like they ought to live in the commitfest entry.

            regards, tom lane

Jelte Fennema-Nio <postgres@jeltef.nl> writes:

> I'd like to send an automatic mail to a thread whenever it gets added

> to a commitfest. Since this would impact everyone that's subscribed to

> the mailinglist I'd like some feedback on this. This mail would

> include:

> 1. A very short blurb like: "This thread was added to the commitfest

> with ID 1234"

> 2. A link to the commitfest entry

> 3. A link to the cfbot CI runs

> 4. A link to the diff on GitHub

> 5. Any other suggestions?

+1 for the basic idea.  An awful lot of people send such mails

manually already, so this'd replace that effort in a more predictable

way.  Not sure about your 3 & 4 points though, especially if that'd

mean some delay in sending the mail.  For one thing, wouldn't those

links be stale as soon as the initial patch got replaced?  Those

links seem like they ought to live in the commitfest entry.

On Thu, 15 Aug 2024 at 01:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> +1 for the basic idea.

That's great to hear.

>  Not sure about your 3 & 4 points though, especially if that'd
> mean some delay in sending the mail.  For one thing, wouldn't those
> links be stale as soon as the initial patch got replaced?

I recently made those links predictable based on only the ID of the CF
entry. So they won't get stale, and I would want to send them straight
away (even if that means that they might return a 404 until cfbot
actually pushes the patches to github). The links would be:

https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf/$CFENTRYID

https://github.com/postgresql-cfbot/postgresql/compare/cf/$CFENTRYID~1...cf/$CFENTRYID

> Those links seem like they ought to live in the commitfest entry.

Agreed. I'll start with that, since that should be very easy.

On Thu, 15 Aug 2024 at 05:17, Euler Taveira <euler@eulerto.com> wrote:
> +1. Regarding the CI link, I would be good if the CF entry automatically adds a
> link to the CI run. It can be a separate field or even add it to "Links".

I'm on it. I think this email should be a subset of the info on the CF
entry webpage, so I'll first change the cf entry page to include all
this info.

> I'm not sure about 4, you can always check the latest patch in the CF entry (it
> is usually the "latest attachment") and that's what the cfbot uses to run.

This is definitely a personal preference thing, but I like reading
patches on GitHub much better than looking at raw patch files. It has
syntax highlighting and has those little arrow buttons at the top of a
diff, to show more context about the file.

I realized a 5th thing that I would want in the email and cf entry page

5. A copy-pastable set of git command that checks out the patch by
downloading it from the cfbot repo like this:

git config branch.cf/5107.remote
https://github.com/postgresql-cfbot/postgresql.git
git config branch.cf/5107.merge refs/heads/cf/5107
git checkout -b cf/5107
git pull

> If I understand your proposal correctly, there will be another email to the
> thread if the previous CF was closed and someone opened a new CF entry.
> Sometimes some CF entries are about the same thread.

Yeah, that's correct. If a new CF entry is created for an existing
thread a new email would be sent. But to be clear, if CF entry is
pushed to the next commitfest, **no** new email would be sent.

On 15.08.24 00:40, Jelte Fennema-Nio wrote:
> I'd like to send an automatic mail to a thread whenever it gets added
> to a commitfest. Since this would impact everyone that's subscribed to
> the mailinglist I'd like some feedback on this. This mail would
> include:
> 
> 1. A very short blurb like: "This thread was added to the commitfest
> with ID 1234"

How would you attach such an email to a thread?  Where in the thread 
would you attach it?  I'm not quite sure how well that would work.

> The main reason I'm proposing this is that currently it's not trivial
> to go from an email in my inbox to the commitfest entry.

Maybe there could be a feature in the commitfest app to enter a message 
ID and get the commitfest entries that contain that message.

On 15.08.24 09:59, Jelte Fennema-Nio wrote:
> I realized a 5th thing that I would want in the email and cf entry page
> 
> 5. A copy-pastable set of git command that checks out the patch by
> downloading it from the cfbot repo like this:
> 
> git config branch.cf/5107.remote
> https://github.com/postgresql-cfbot/postgresql.git
> git config branch.cf/5107.merge refs/heads/cf/5107
> git checkout -b cf/5107
> git pull

Maybe this kind of thing should rather be on the linked-to web page, not 
in every email.

But a more serious concern here is that the patches created by the cfbot 
are not canonical.  There are various heuristics when they get applied. 
I would prefer that people work with the actual patches sent by email, 
at least unless they know exactly what they are doing.  We don't want to 
create parallel worlds of patches that are like 90% similar but not 
really identical.

On Thu, 15 Aug 2024 at 15:28, Peter Eisentraut <peter@eisentraut.org> wrote:
> How would you attach such an email to a thread?  Where in the thread
> would you attach it?  I'm not quite sure how well that would work.

My idea would be to have the commitfest app send it in reply to the
message id that was entered in the "New patch" page:
https://commitfest.postgresql.org/open/new/

> Maybe there could be a feature in the commitfest app to enter a message
> ID and get the commitfest entries that contain that message.

That's a good idea.

On Thu, 15 Aug 2024 at 15:33, Peter Eisentraut <peter@eisentraut.org> wrote:
> Maybe this kind of thing should rather be on the linked-to web page, not
> in every email.

Yeah, I'll first put a code snippet on the page for the commitfest entry.

> But a more serious concern here is that the patches created by the cfbot
> are not canonical.  There are various heuristics when they get applied.
> I would prefer that people work with the actual patches sent by email,
> at least unless they know exactly what they are doing.  We don't want to
> create parallel worlds of patches that are like 90% similar but not
> really identical.

I'm not really sure what kind of heuristics and resulting differences
you're worried about here. The heuristics it uses are very simple and
are good enough for our CI. Basically they are:
1. Unzip/untar based on file extension
2. Apply patches using "patch" in alphabetic order

Also, when I apply patches myself, I use heuristics too. And my
heuristics are probably different from yours. So I'd expect that many
people using the exact same heuristic would only make the situation
better. Especially because if people don't know exactly what they are
doing, then their heuristics are probably not as good as the one of
our cfbot. I know I've struggled a lot the first few times when I was
manually applying patches.

On Thu, Aug 15, 2024 at 12:40:29AM +0200, Jelte Fennema-Nio wrote:
> I'd like to send an automatic mail to a thread whenever it gets added
> to a commitfest. Since this would impact everyone that's subscribed to
> the mailinglist I'd like some feedback on this. This mail would
> include:

I haven't thought too much about the details, but in general, +1 for
sending links to cfest/cfbot entries to the thread.

-- 
nathan

On 15.08.24 19:25, Matthias van de Meent wrote:
> Apart from the above issue, I'm -0.5 on what to me equates with 
> automated spam to -hackers: the volume of mails would put this around 
> the 16th most common sender on -hackers, with about 400 mails/year 
> (based on 80 new patches for next CF, and 5 CFs/year, combined with 
> Robert's 2023 statistics at [0]).

Yeah, I'd rather not open the can of worms that we send automated emails 
to this list at all.  If we do this, then there will be other requests, 
and why this one and not that one.  If people want to get emails from 
the commitfest app, it should be that you subscribe there and it sends 
those emails to those who want them.

> I also don't quite like the suggested contents of such mail: (1) and (2) 
> are essentially duplicative information, and because CF's entries' IDs 
> are not shown in the app the "with ID 0000" part of (1) is practically 
> useless (better use the CFE's title), (3) would best be stored and/or 
> integrated in the CFA, as would (4). Additionally, (4) isn't 
> canonical/guaranteed to be up-to-date, see above. As for the 
> "copy-pastable git commands" suggestion, I'm not sure that's applicable, 
> for the same reasons that (4) won't work reliably. CFBot's repo to me 
> seems more like an internal implementation detail of CFBot than an 
> authorative source of patchset diffs.

I agree.  And this also smells a bit like "my favorite workflow".  Maybe 
start with a blog post or a wiki page if you want to suggest this.

Peter Eisentraut <peter@eisentraut.org> writes:
> On 15.08.24 19:25, Matthias van de Meent wrote:
>> Apart from the above issue, I'm -0.5 on what to me equates with 
>> automated spam to -hackers: the volume of mails would put this around 
>> the 16th most common sender on -hackers, with about 400 mails/year 
>> (based on 80 new patches for next CF, and 5 CFs/year, combined with 
>> Robert's 2023 statistics at [0]).

> Yeah, I'd rather not open the can of worms that we send automated emails 
> to this list at all.  If we do this, then there will be other requests, 
> and why this one and not that one.  If people want to get emails from 
> the commitfest app, it should be that you subscribe there and it sends 
> those emails to those who want them.

That would destroy the one good argument for this, which was to
provide an easy way to get from a mail list thread (in the archives)
to the corresponding CF entry or entries.  You pull up the "flat"
thread, search for the bot mail, and click the link.  Without that
I see little point at all.

However, there are other ways to accomplish that.  I liked the
suggestion of extending the CF webapp with a way to search for entries
mentioning a particular mail message ID.  I dunno how hard it'd be to
get it to recognize *any* message-ID from a thread, but surely at
least the head message ought not be too hard to match.

            regards, tom lane

On Wed, Aug 14, 2024 at 3:40 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> I'd like to send an automatic mail to a thread whenever it gets added
> to a commitfest. Since this would impact everyone that's subscribed to
> the mailinglist I'd like some feedback on this. This mail would
> include:
>
> 1. A very short blurb like: "This thread was added to the commitfest
> with ID 1234"
> 2. A link to the commitfest entry
> 3. A link to the cfbot CI runs
> 4. A link to the diff on GitHub
> 5. Any other suggestions?

I would find (2) very useful.

On Fri, Aug 16, 2024 at 10:23 AM Maciek Sakrejda <maciek@pganalyze.com> wrote:
> > 1. A very short blurb like: "This thread was added to the commitfest
> > with ID 1234"
> > 2. A link to the commitfest entry
> > 3. A link to the cfbot CI runs
> > 4. A link to the diff on GitHub
> > 5. Any other suggestions?
>
> I would find (2) very useful.

Me too.

--Jacob

On Thu, Aug 15, 2024 at 9:33 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> But a more serious concern here is that the patches created by the cfbot
> are not canonical.  There are various heuristics when they get applied.

It's true that the code required for CFBot to simply apply a patch is
nontrivial. We're accounting for various edge-cases, and soldiering
on, where we deem that it makes sense. I'm referring to those cases
where "git am" won't work perfectly, but we have a pretty good chance
of successfully generating what the patch author intended (I assume
that that's what you meant by "heuristics").

One reason why this works better than might be expected is
because...we then test the result (that's the whole point, of course).
Obviously, if we apply a patch in a way that isn't quite perfectly
clean (according to whatever the "git am" criteria is), but that CFBot
is nevertheless capable of applying, and we then find that the end
result passes all tests, that gives us a fairly strong signal. We can
have high confidence that CFBot has done the right thing at that
point. We can reasonably present the resulting feature branch as a
"known good" usable version of the patch.

Of course you can quibble with this. Fundamentally, a patch file can
never be updated, but we want to apply it on top of a moving target
(as best we can). We're always making some kind of trade-off. I just
don't think that the heuristics that humans might choose to apply are
necessarily much better on average. Humans are bad at routine boring
things, but good at noticing and coping with special cases.

> I would prefer that people work with the actual patches sent by email,
> at least unless they know exactly what they are doing.  We don't want to
> create parallel worlds of patches that are like 90% similar but not
> really identical.

There's a reason why tech companies place so much emphasis on offering
a "low friction experience". The aggregate effect on user/consumer
behavior is huge. I'm not saying that this is good or bad (let's not
get into that now); just that it is an empirical fact that people tend
to behave like that. We want more reviewers. Why not try to meet
people where they are a bit more?

I have to admit that I think that I'd be far more likely to quickly
test out a patch if I'm presented with a workflow that makes the setup
as painless as possible. Particularly if I'm all but guaranteed to get
a working Postgres server with the patch applied (which is possible
when I'm building exactly the same feature branch as the one that
passed CI by CFBot). I'm entirely willing to believe that it wouldn't
work that way for you, but I'm confident that a lot of people are in
the same camp as me.

--
Peter Geoghegan

On Thu, 15 Aug 2024 at 10:40, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> I'd like to send an automatic mail to a thread whenever it gets added
> to a commitfest. Since this would impact everyone that's subscribed to
> the mailinglist I'd like some feedback on this. This mail would
> include:
>
> 1. A very short blurb like: "This thread was added to the commitfest
> with ID 1234"
> 2. A link to the commitfest entry

One thing I have seen which #2 would have helped with was that during
a CF, a patch author posted a trivial patch to -hackers and I
committed it. I didn't know the patch author had added a CF entry to
the *next* CF. I seldom think to look at the next CF page during a CF.
The entry was closed by the patch author in this case.

For that reason, I think #2 would be useful.

> 3. A link to the cfbot CI runs
> 4. A link to the diff on GitHub
> 5. Any other suggestions?

I don't really object, but I think a better aim would be to merge
CFbot with CF so that we could get to those places from the CF entry.

David

On Sat, Aug 17, 2024 at 11:36 AM David Rowley <dgrowleyml@gmail.com> wrote:
> I don't really object, but I think a better aim would be to merge
> CFbot with CF so that we could get to those places from the CF entry.

Ack.  Some progress is happening on that front, working with Jelte and
Magnus off-list...

    Jelte Fennema-Nio wrote:

> I'd like to send an automatic mail to a thread whenever it gets added
> to a commitfest

Instead of sending a specific mail, what about automatically adding a
mail header like:

  X-CommitFest-Entry: <https://commitfest.postgresql.org/X/Y>

to every outgoing mail whose thread is associated to a CF entry?

It feels more principled and less "in-your-face" than one dedicated
message, plus it would work for all the patches that are already
registered.

On the web archive, the displayer could show it as a clickable link,
along with the main mail headers (from, to, date, Message-ID...), if
 it's deemed important enough to make it prominent.
Otherwise one can click on "Raw Message" to see all headers.


Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite

On Fri, Aug 16, 2024 at 3:13 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> Yeah, I'd rather not open the can of worms that we send automated emails
> to this list at all.

+1.

> If people want to get emails from
> the commitfest app, it should be that you subscribe there and it sends
> those emails to those who want them.

+1.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Fri, 16 Aug 2024 at 16:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> However, there are other ways to accomplish that.  I liked the
> suggestion of extending the CF webapp with a way to search for entries
> mentioning a particular mail message ID.  I dunno how hard it'd be to
> get it to recognize *any* message-ID from a thread, but surely at
> least the head message ought not be too hard to match.

I sent a patch to support this on the commitfest app to Magnus
off-list. It was pretty easy to implement, even for *any* message-ID.

Jelte Fennema-Nio <postgres@jeltef.nl> writes:
> On Fri, 16 Aug 2024 at 16:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> However, there are other ways to accomplish that.  I liked the
>> suggestion of extending the CF webapp with a way to search for entries
>> mentioning a particular mail message ID.  I dunno how hard it'd be to
>> get it to recognize *any* message-ID from a thread, but surely at
>> least the head message ought not be too hard to match.

> I sent a patch to support this on the commitfest app to Magnus
> off-list. It was pretty easy to implement, even for *any* message-ID.

Cool, thank you!

            regards, tom lane

On Sat, Aug 17, 2024 at 9:44 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sat, Aug 17, 2024 at 11:36 AM David Rowley <dgrowleyml@gmail.com> wrote:
> > I don't really object, but I think a better aim would be to merge
> > CFbot with CF so that we could get to those places from the CF entry.

I've built a system for pushing all the data required to show the
cfbot traffic lights over to the CF app, and shared it with Magnus to
see if he likes it.  If so, hopefully some more progress soon...

So, there was limited enthusiasm for a new message here, but I noticed
that current CF messages don't include the CF entry link [1]. What
about adding a link to its existing e-mails?

Thanks,
Maciek

[1]: e.g., https://www.postgresql.org/message-id/172579582925.1126.2395496302629349708.pgcf%40coridan.postgresql.org

On Mon, Aug 26, 2024 at 9:39 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> I've built a system for pushing all the data required to show the
> cfbot traffic lights over to the CF app, and shared it with Magnus to
> see if he likes it.  If so, hopefully some more progress soon...

I don't see any traffic lights on commitfest.postgresql.org yet.
Should I #blamemagnus?

--
Robert Haas
EDB: http://www.enterprisedb.com

On Thu, 15 Aug 2024 at 20:00, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> On Thu, 15 Aug 2024 at 05:17, Euler Taveira <euler@eulerto.com> wrote:
> > If I understand your proposal correctly, there will be another email to the
> > thread if the previous CF was closed and someone opened a new CF entry.
> > Sometimes some CF entries are about the same thread.
>
> Yeah, that's correct. If a new CF entry is created for an existing
> thread a new email would be sent. But to be clear, if CF entry is
> pushed to the next commitfest, **no** new email would be sent.

I was thinking about this today when looking at [1], you can see at
the end of that email someone posted a link to the CF entry.
Unfortunately, that was for the Jan-2024 CF, which is not really that
useful to look at today.

It looks like only about 35% of patches in this CF have "num CFs" = 1,
so it might be annoying to be taken to an old entry 65% of the time
you click the proposed URL. Maybe instead of the URLs showing the CF
number that the patch was first added to, if it could have a reference
in the URL that looks up the maximum CF which contains the patch and
show that one. e.g. https://commitfest.postgresql.org/latest/4751/ .
Without that, if you try to modify the status of a patch in an older
CF, you'll just be told that you can't do that.  (it is true you can
click the latest CF in the status column, but that's at least one more
click than I'd have hoped)

David

[1] https://postgr.es/m/CACJufxEdavJhkUDhJ1jraXnZ9ayNQU+TvjuQjzQbuGS06oNZEQ@mail.gmail.com

On Thu, 12 Sept 2024 at 04:07, David Rowley <dgrowleyml@gmail.com> wrote:
> Maybe instead of the URLs showing the CF
> number that the patch was first added to, if it could have a reference
> in the URL that looks up the maximum CF which contains the patch and
> show that one. e.g. https://commitfest.postgresql.org/latest/4751/ .

Yeah agreed, that's why I added support for
https://commitfest.postgresql.org/patch/4751/ a while ago. That URL
will be easily discoverable on the commitfest entry page soon (patch
for this is in flight).