Thread: [PATCH] CF app: add "Returned: Needs more interest"

[PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
[Trying -hackers rather than -www this time, since the impacted users are here.]

There are occasionally patches that are dutifully rebased by a
responsive author, all feedback implemented... but there have been no
reviews for a while, and there's no sign of any on the way. This case
seems to tie us up in knots. We don't want to Reject since the patch
is fine, it's just not a current priority; and we don't want to Return
with Feedback because there is no feedback to act upon. Since no one
wants to close it, they can drag on forever, with hopeful authors
rebasing eternally. I ended up closing several patches like this with
RwF, but I felt the need to write a huge explanation in the
accompanying email.

This has been discussed before (e.g. [1]); the two competing proposals
I've seen are to add a new close state or to simply remove the "with
Feedback" and treat all Returned patches the same. I can see the case
for minimizing the number of choices, but in this patchset, I've opted
to implement a new state. I think it's useful to communicate to a
contributor that their next steps, if they still want to pursue them,
need to be focused on coalition building rather than code changes. And
I think Returned with Feedback has a useful meaning already.

0001 just adds the "Returned: Needs more interest" state to the
database and makes it available to the UI. The (optional) 0002 goes
farther, and puts the two Returned states along with the Next CF state
into a new "Deferred" section in the UI. (It's UI-only; the app still
treats them as closed patches for the purposes of the CF.) This
emphasizes the distinction between Returned and Rejected: Rejected is
meant to be a full stop to the story, while Returned is more of a
comma.

Screenshots attached. WDYT?

--Jacob

[1] https://www.postgresql.org/message-id/flat/3905363.1633288498%40sss.pgh.pa.us

Attachment

Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Julien Rouhaud
Date:
Hi,

On Tue, Aug 02, 2022 at 03:55:28PM -0700, Jacob Champion wrote:
> [Trying -hackers rather than -www this time, since the impacted users are here.]
>
> There are occasionally patches that are dutifully rebased by a
> responsive author, all feedback implemented... but there have been no
> reviews for a while, and there's no sign of any on the way. This case
> seems to tie us up in knots. We don't want to Reject since the patch
> is fine, it's just not a current priority; and we don't want to Return
> with Feedback because there is no feedback to act upon. Since no one
> wants to close it, they can drag on forever, with hopeful authors
> rebasing eternally. I ended up closing several patches like this with
> RwF, but I felt the need to write a huge explanation in the
> accompanying email.
> [...]
> [1] https://www.postgresql.org/message-id/flat/3905363.1633288498%40sss.pgh.pa.us

I'm personally fine with the current statutes, as closing a patch with RwF
explaining that there was no interest is still a feedback, and having a
different status won't make it any more pleasant for both the CFM and the
author.

My biggest complaint here is that it doesn't really do anything to try to
improve the current situation (lack of review and/or lack of committer
interest).

Maybe it would be better to discuss some clear rules and thresholds on when
action should be taken on such patches.  It doesn't have to be closing the CF
entry directly but instead sending some email to ask for community / committer
feedback as in the thread you pointed, and document that in the commitfest wiki
page.



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On Tue, Aug 2, 2022 at 8:00 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> I'm personally fine with the current statutes, as closing a patch with RwF
> explaining that there was no interest is still a feedback,

Hi Julien,

Making that explanation each time we intend to close a patch "needs
interest" takes a lot of time and wordsmithing. "Returned with
feedback" clearly has an established meaning to the community, and
this is counter to that meaning, so people just avoid using it that
way.

When they do, miscommunications happen easily, which can lead to
authors reopening patches thinking that there's been some kind of
mistake (as happened to at least one of the patches in this past CF,
which I had to close again). Language and cultural differences likely
exacerbate the problem, so the less ad hoc messaging a CFM has to do
to explain that "this is RwF but not actually RwF", the better.

> and having a
> different status won't make it any more pleasant for both the CFM and the
> author.

"More pleasant" is not really the goal here. I don't think it should
ever be pleasant for a CFM to return someone's patch when it hasn't
received review, and it's certainly not going to be pleasant for the
author. But we can be more honest and clear about why we're returning
it, and hopefully make it less unpleasant.

> My biggest complaint here is that it doesn't really do anything to try to
> improve the current situation (lack of review and/or lack of committer
> interest).

It's not really meant to improve that. This is just trying to move the
needle a little bit, in a way that's been requested several times.

> Maybe it would be better to discuss some clear rules and thresholds on when
> action should be taken on such patches.

I think that's also important to discuss, and I have thoughts on that
too, but I don't think the discussions for these sorts of incremental
changes should wait for that discussion.

--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Julien Rouhaud
Date:
Hi,

On Wed, Aug 03, 2022 at 08:58:49AM -0700, Jacob Champion wrote:
> On Tue, Aug 2, 2022 at 8:00 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > I'm personally fine with the current statutes, as closing a patch with RwF
> > explaining that there was no interest is still a feedback,
>
> Making that explanation each time we intend to close a patch "needs
> interest" takes a lot of time and wordsmithing. "Returned with
> feedback" clearly has an established meaning to the community, and
> this is counter to that meaning, so people just avoid using it that
> way.
>
> When they do, miscommunications happen easily, which can lead to
> authors reopening patches thinking that there's been some kind of
> mistake (as happened to at least one of the patches in this past CF,
> which I had to close again). Language and cultural differences likely
> exacerbate the problem, so the less ad hoc messaging a CFM has to do
> to explain that "this is RwF but not actually RwF", the better.
>
> > and having a
> > different status won't make it any more pleasant for both the CFM and the
> > author.
>
> "More pleasant" is not really the goal here. I don't think it should
> ever be pleasant for a CFM to return someone's patch when it hasn't
> received review, and it's certainly not going to be pleasant for the
> author. But we can be more honest and clear about why we're returning
> it, and hopefully make it less unpleasant.
>
> > My biggest complaint here is that it doesn't really do anything to try to
> > improve the current situation (lack of review and/or lack of committer
> > interest).
>
> It's not really meant to improve that. This is just trying to move the
> needle a little bit, in a way that's been requested several times.
>
> > Maybe it would be better to discuss some clear rules and thresholds on when
> > action should be taken on such patches.
>
> I think that's also important to discuss, and I have thoughts on that
> too, but I don't think the discussions for these sorts of incremental
> changes should wait for that discussion.

First of all, I didn't want to imply that rejecting a patch should be pleasant,
sorry if that sounded that way.

It's not that I'm opposed to adding that status, I just don't see how it's
really going to improve the situation on its own.  Or maybe because it wouldn't
make any difference to me as a patch author to get my patches returned "with
feedback" or "for any other reason" if they are ignored.  I'm afraid that
patches will still be left alone to rot and there still be no clear rules on
what to do and when, reminder for CFM and such, and that this new status would
never be used anyway.  So I guess I will just stop hijacking this thread and
wait for a discussion on that, sorry for the noise.



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On Wed, Aug 3, 2022 at 10:09 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> First of all, I didn't want to imply that rejecting a patch should be pleasant,
> sorry if that sounded that way.

No worries, I don't think it really sounded that way. :D

> It's not that I'm opposed to adding that status, I just don't see how it's
> really going to improve the situation on its own.

If the situation you're referring to is the fact that we have a lot of
patches sitting without review, it won't improve that situation, I
agree.

The situation I'm looking at, though, is where we have a dozen patches
floating forward that multiple CFMs in a row feel should be returned,
but they won't because claiming "they have feedback" is clearly unfair
to the author. I think this will improve that situation.

> Or maybe because it wouldn't
> make any difference to me as a patch author to get my patches returned "with
> feedback" or "for any other reason" if they are ignored.

Sure. I think this change helps the newer contributors (and the CFMs
talking to them) more than it helps the established ones.

I'm in your boat, where I don't personally care how a patch of mine is
returned (and I'm fine with Withdrawing them myself). But I'm also
paid to do this. From some of my past experiences with other projects,
I tend to feel more sensitive to bad communication if I've developed a
patch using volunteer hours, on evenings or weekends.

> I'm afraid that
> patches will still be left alone to rot and there still be no clear rules on
> what to do and when, reminder for CFM and such, and that this new status would
> never be used anyway.  So I guess I will just stop hijacking this thread and
> wait for a discussion on that, sorry for the noise.

Well, here, let's keep that conversation going too while there's
momentum. One sec while I switch Subjects and continue...

--Jacob



Clarifying Commitfest policies

From
Jacob Champion
Date:
[was: CF app: add "Returned: Needs more interest"]

On Wed, Aug 3, 2022 at 10:09 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> I'm afraid that
> patches will still be left alone to rot and there still be no clear rules on
> what to do and when, reminder for CFM and such, and that this new status would
> never be used anyway.

Yeah, so the lack of clear rules is an issue -- maybe not because we
can't work without them (we have, clearly, and we can continue to do
so) but because each of us kind of makes it up as we go along? When
discussions about these "rules" happen on the list, it doesn't always
happen with the same people, and opinions can vary wildly.

There have been a couple of suggestions recently:
- Revamp the CF Checklist on the wiki. I plan to do so later this
month, but that will still need some community review.
- Provide in-app explanations and documentation for some of the less
obvious points. (What should the target version be? What's the
difference between Rejected and Returned?)

Is that enough, or should we do more?

My preference, as I think Daniel also said in a recent thread, would
be for most of this information to be in the application somewhere.
That would make it more immediately accessible to everyone. (The
tradeoff is, it gets harder to update.)

--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Andres Freund
Date:
Hi,

On 2022-08-03 10:52:36 -0700, Jacob Champion wrote:
> The situation I'm looking at, though, is where we have a dozen patches
> floating forward that multiple CFMs in a row feel should be returned,
> but they won't because claiming "they have feedback" is clearly unfair
> to the author. I think this will improve that situation.

What patches are we concretely talking about?

My impression is that a lot of the patches floating from CF to CF have gotten
sceptical feedback and at best a minor amount of work to address that has been
done.

Greetings,

Andres Freund



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> My impression is that a lot of the patches floating from CF to CF have gotten
> sceptical feedback and at best a minor amount of work to address that has been
> done.

That I think is a distinct issue: nobody wants to take on the
unpleasant job of saying "no, we don't want this" in a final way.
We may raise some objections but actually rejecting a patch is hard.
So it tends to slide forward until the author gives up.

            regards, tom lane



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On 8/3/22 11:41, Andres Freund wrote:
> What patches are we concretely talking about?>
> My impression is that a lot of the patches floating from CF to CF have gotten
> sceptical feedback and at best a minor amount of work to address that has been
> done.

- https://commitfest.postgresql.org/38/2482/
- https://commitfest.postgresql.org/38/3338/
- https://commitfest.postgresql.org/38/3181/
- https://commitfest.postgresql.org/38/2918/
- https://commitfest.postgresql.org/38/2710/
- https://commitfest.postgresql.org/38/2266/ (this one was particularly
miscommunicated during the first RwF)
- https://commitfest.postgresql.org/38/2218/
- https://commitfest.postgresql.org/38/3256/
- https://commitfest.postgresql.org/38/3310/
- https://commitfest.postgresql.org/38/3050/

Looking though, some of those have received skeptical feedback as you
say, but certainly not all; not even a majority IMO. (Even if they'd all
received skeptical feedback, if the author replies in good faith and is
met with silence for months, we need to not keep stringing them along.)

--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Andres Freund
Date:
Hi,

On 2022-08-03 12:06:03 -0700, Jacob Champion wrote:
> On 8/3/22 11:41, Andres Freund wrote:
> > What patches are we concretely talking about?>
> > My impression is that a lot of the patches floating from CF to CF have gotten
> > sceptical feedback and at best a minor amount of work to address that has been
> > done.
> 
> - https://commitfest.postgresql.org/38/2482/

Hm - "Returned: Needs more interest" doesn't seem like it'd have been more
descriptive? It was split off a patchset that was committed at the tail end of
15 (and which still has *severe* code quality issues). Imo having a CF entry
before the rest of the jsonpath stuff made it in doesn't seem like a good
idea.


> - https://commitfest.postgresql.org/38/3338/

Here it'd have fit.


> - https://commitfest.postgresql.org/38/3181/

FWIW, I mentioned at least once that I didn't think this was worth pursuing.


> - https://commitfest.postgresql.org/38/2918/

Hm, certainly not a lot of review activity.


> - https://commitfest.postgresql.org/38/2710/

A good bit of this was committed in some form with a decent amount of review
activity for a while.


> - https://commitfest.postgresql.org/38/2266/ (this one was particularly
> miscommunicated during the first RwF)

I'd say misunderstanding than miscommunication...

It seems partially stalled due to the potential better approach based on
https://www.postgresql.org/message-id/flat/15848.1576515643%40sss.pgh.pa.us ?
In which case RwF doesn't seem to inappropriate.


> - https://commitfest.postgresql.org/38/2218/

Yep.


> - https://commitfest.postgresql.org/38/3256/

Yep.


> - https://commitfest.postgresql.org/38/3310/

I don't really understand why this has been RwF'd, doesn't seem that long
since the last review leading to changes.


> - https://commitfest.postgresql.org/38/3050/

Given that a non-author did a revision of the patch, listed a number of TODO
items and said "I'll create regression tests firstly." - I don't think "lacks
interest" would have been appropriate, and RwF is?


> (Even if they'd all received skeptical feedback, if the author replies in
> good faith and is met with silence for months, we need to not keep stringing
> them along.)

I agree very much with that - just am doubtful that "lacks interest" is a good
way of dealing with it, unless we just want to treat it as a nicer sounding
"rejected".

Greetings,

Andres Freund



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> I agree very much with that - just am doubtful that "lacks interest" is a good
> way of dealing with it, unless we just want to treat it as a nicer sounding
> "rejected".

I think there is a difference.  "Lacks interest" suggests that there
is a path forward for the patch, namely (as Jacob has mentioned
repeatedly) doing some sort of consensus-building that it's worth
pursuing.  The author may or may not have the interest/skills to do
that, but it's possible that it could happen.  "Rejected" says "don't
bother pursuing this, it's a bad idea".  Neither of these seems the
same as RWF, which I think we mostly understand to mean "this patch
has technical problems that can probably be fixed".

            regards, tom lane



Re: Clarifying Commitfest policies

From
Matthias van de Meent
Date:
On Wed, 3 Aug 2022 at 20:04, Jacob Champion <jchampion@timescale.com> wrote:
>
> [was: CF app: add "Returned: Needs more interest"]
>
> On Wed, Aug 3, 2022 at 10:09 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > I'm afraid that
> > patches will still be left alone to rot and there still be no clear rules on
> > what to do and when, reminder for CFM and such, and that this new status would
> > never be used anyway.
>
> Yeah, so the lack of clear rules is an issue -- maybe not because we
> can't work without them (we have, clearly, and we can continue to do
> so) but because each of us kind of makes it up as we go along? When
> discussions about these "rules" happen on the list, it doesn't always
> happen with the same people, and opinions can vary wildly.
>
> There have been a couple of suggestions recently:
> - Revamp the CF Checklist on the wiki. I plan to do so later this
> month, but that will still need some community review.
> - Provide in-app explanations and documentation for some of the less
> obvious points. (What should the target version be? What's the
> difference between Rejected and Returned?)
>
> Is that enough, or should we do more?

"The CF Checklist" seems to refer to only the page that is (or seems
to be) intended for the CFM only. We should probably also update the
pages of "Commitfest", "Submitting a patch", "Reviewing a Patch", "So,
you want to be a developer?", and the "Developer FAQ" page, which
doesn't have to be more than removing outdated information and
refering to any (new) documentation on how to participate in the
PostgreSQL development and/or Commitfest workflow as a non-CFM.

Additionally, we might want to add extra text to the "developers"
section of the main website [0] to refer to any new documentation.
This suggestion does depend on whether the new documentation has a
high value for potential community members.

Lastly, a top-level CONTRIBUTING.md file in git repositories is also
often used as an entry point for potential contributors. I don't
suggest we copy all documentation into the main repo, just that a
pointer to our existing contributer entry documentation in such a file
could help lower the barrier of entry.
As an example, the GitHub mirror of the main PostgreSQL repository
receives a decent amount of pull request traffic. When a project has a
CONTRIBUTING.md -file at the top level people writing the pull request
message will be pointed to those contributing guidelines. This could

Thank you for raising this to a topical thread.

Kind regards,

Matthias van de Meent

[0] https://www.postgresql.org/developer/



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
Hi Andres,

My intention had not quite been for this to be a referendum on the
decision for every patch -- we can do that if it helps, but I don't
think we necessarily have to have unanimity on the bucketing for every
patch in order for the new state to be useful.

On 8/3/22 12:46, Andres Freund wrote:
>> - https://commitfest.postgresql.org/38/2482/
> 
> Hm - "Returned: Needs more interest" doesn't seem like it'd have been more
> descriptive? It was split off a patchset that was committed at the tail end of
> 15 (and which still has *severe* code quality issues). Imo having a CF entry
> before the rest of the jsonpath stuff made it in doesn't seem like a good
> idea
There were no comments about code quality issues on the thread that I
can see, and there were three people who independently said "I don't
know why this isn't getting review." Seems like a shoe-in for "needs
more interest".

>> - https://commitfest.postgresql.org/38/3338/
> 
> Here it'd have fit.

Okay. That's one.

>> - https://commitfest.postgresql.org/38/3181/
> 
> FWIW, I mentioned at least once that I didn't think this was worth pursuing.

(I don't see that comment on that thread? You mentioned it needed a rebase.)

IMO, mentioning that something is not worth pursuing is not actionable
feedback. It's a declaration of non-interest in the mildest case, and a
Rejection in the strongest case. But let's please not say "meh" and then
Return with Feedback; an author can't do anything with that.

>> - https://commitfest.postgresql.org/38/2918/
> 
> Hm, certainly not a lot of review activity.

That's two.

>> - https://commitfest.postgresql.org/38/2710/
> 
> A good bit of this was committed in some form with a decent amount of review
> activity for a while.

But then the rest of it stalled. Something has to be done with the open
entry.

>> - https://commitfest.postgresql.org/38/2266/ (this one was particularly
>> miscommunicated during the first RwF)
> 
> I'd say misunderstanding than miscommunication...

The CFM sending it said, "It seems there has been no activity since last
version of the patch so I don't think RwF is correct" [1], and then the
email sent said "you are encouraged to send a new patch [...] with the
suggested changes." But there were no suggested changes left to make.

This really highlights, for me, why the two states should not be
combined into one.

> It seems partially stalled due to the potential better approach based on
> https://www.postgresql.org/message-id/flat/15848.1576515643%40sss.pgh.pa.us ?
> In which case RwF doesn't seem to inappropriate.

Those comments are, as far as I can tell, not in the thread. (And the
new thread you linked is also stalled.)

>> - https://commitfest.postgresql.org/38/2218/
> 
> Yep.

That's three.

>> - https://commitfest.postgresql.org/38/3256/
> 
> Yep.

That's four.

>> - https://commitfest.postgresql.org/38/3310/
> 
> I don't really understand why this has been RwF'd, doesn't seem that long
> since the last review leading to changes.

Eight months without feedback, when we expect authors to turn around a
patch in two weeks or less to avoid being RwF'd, is a long time IMHO. I
don't think a patch should sit motionless in CF for eight months; it's
not at all fair to the author.

>> - https://commitfest.postgresql.org/38/3050/
> 
> Given that a non-author did a revision of the patch, listed a number of TODO
> items and said "I'll create regression tests firstly." - I don't think "lacks
> interest" would have been appropriate, and RwF is?

That was six months ago, and prior to that there was another six month
silence. I'd say that lacks interest, and I don't feel like it's
currently reviewable in CF.

>> (Even if they'd all received skeptical feedback, if the author replies in
>> good faith and is met with silence for months, we need to not keep stringing
>> them along.)
> 
> I agree very much with that - just am doubtful that "lacks interest" is a good
> way of dealing with it, unless we just want to treat it as a nicer sounding
> "rejected".
Tom summed up my position well: there's a difference between those two
that is both meaningful and actionable for contributors. Is there an
alternative you'd prefer?

Thanks for the discussion!
--Jacob

[1] https://www.postgresql.org/message-id/20211004071249.GA6304%40ahch-to




Re: Clarifying Commitfest policies

From
Jacob Champion
Date:
On Wed, Aug 3, 2022 at 2:05 PM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> On Wed, 3 Aug 2022 at 20:04, Jacob Champion <jchampion@timescale.com> wrote:
> > Is that enough, or should we do more?
>
> "The CF Checklist" seems to refer to only the page that is (or seems
> to be) intended for the CFM only. We should probably also update the
> pages of "Commitfest", "Submitting a patch", "Reviewing a Patch", "So,
> you want to be a developer?", and the "Developer FAQ" page, which
> doesn't have to be more than removing outdated information and
> refering to any (new) documentation on how to participate in the
> PostgreSQL development and/or Commitfest workflow as a non-CFM.

Agreed, a sweep of those materials would be helpful as well. I'm
personally focused on CFM tasks, since it's fresh in my brain and
documentation is almost non-existent for it, but if you have ideas for
those areas, I definitely don't want to shut down that line of the
conversation.

> Additionally, we might want to add extra text to the "developers"
> section of the main website [0] to refer to any new documentation.
> This suggestion does depend on whether the new documentation has a
> high value for potential community members.

Right. So what kinds of info do we want to highlight in this
documentation, to make it high-quality?

Drawing from some of the questions I've seen recently, we could talk about
- CF "power" structure (perhaps simply to highlight that the CFM has
no additional authority to get patches in)
- the back-and-forth process on the mailing list, maybe including
expected response times
- what to do when a patch is returned (or rejected)

> As an example, the GitHub mirror of the main PostgreSQL repository
> receives a decent amount of pull request traffic. When a project has a
> CONTRIBUTING.md -file at the top level people writing the pull request
> message will be pointed to those contributing guidelines. This could

(I think some text got cut here.)

The mirror bot will point you to the "So, you want to be a developer?"
wiki when you open a PR, but I agree that a CONTRIBUTING doc would
help prevent that small embarrassment.

--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Andres Freund
Date:
Hi,

On 2022-08-04 11:19:28 -0700, Jacob Champion wrote:
> My intention had not quite been for this to be a referendum on the
> decision for every patch -- we can do that if it helps, but I don't
> think we necessarily have to have unanimity on the bucketing for every
> patch in order for the new state to be useful.

Sorry, I should have been clearer. It wasn't mine either! I was just trying to
understand what you see as the usecase / get a better feel for it. I'm now a
bit more convinced it's useful than before.


> >> - https://commitfest.postgresql.org/38/3310/
> > 
> > I don't really understand why this has been RwF'd, doesn't seem that long
> > since the last review leading to changes.
> 
> Eight months without feedback, when we expect authors to turn around a
> patch in two weeks or less to avoid being RwF'd, is a long time IMHO.

Why is it better to mark it as lacks interested than RwF if there actually
*has* been feedback?


> I don't think a patch should sit motionless in CF for eight months; it's not
> at all fair to the author.

It's not great, I agree, but wishes don't conjure up resources :(


> >> - https://commitfest.postgresql.org/38/3050/
> > 
> > Given that a non-author did a revision of the patch, listed a number of TODO
> > items and said "I'll create regression tests firstly." - I don't think "lacks
> > interest" would have been appropriate, and RwF is?
> 
> That was six months ago, and prior to that there was another six month
> silence. I'd say that lacks interest, and I don't feel like it's
> currently reviewable in CF.

I don't think the entry needs more review - it needs changes:
https://www.postgresql.org/message-id/CAOKkKFtc45uNFoWYOCo4St19ayxrh-_%2B4TnZtwxGZz6-3k_GSA%40mail.gmail.com
That contains quite a few things that should be changed.

A patch that has gotten feedback, but that feedback hasn't been processed
pretty much is the definition of RwF, no?


> >> (Even if they'd all received skeptical feedback, if the author replies in
> >> good faith and is met with silence for months, we need to not keep stringing
> >> them along.)
> > 
> > I agree very much with that - just am doubtful that "lacks interest" is a good
> > way of dealing with it, unless we just want to treat it as a nicer sounding
> > "rejected".
>
> Tom summed up my position well: there's a difference between those two
> that is both meaningful and actionable for contributors. Is there an
> alternative you'd prefer?

I agree that "lacks interest" could be useful. But I'm wary of it becoming
just a renaming if we end up marking patches that should be RwF or rejected as
"lacks interest".

Greetings,

Andres Freund



Re: Clarifying Commitfest policies

From
Matthias van de Meent
Date:
On Thu, 4 Aug 2022 at 20:38, Jacob Champion <jchampion@timescale.com> wrote:
>
> On Wed, Aug 3, 2022 at 2:05 PM Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> > On Wed, 3 Aug 2022 at 20:04, Jacob Champion <jchampion@timescale.com> wrote:
> > > Is that enough, or should we do more?
> >
> > "The CF Checklist" seems to refer to only the page that is (or seems
> > to be) intended for the CFM only. We should probably also update the
> > pages of "Commitfest", "Submitting a patch", "Reviewing a Patch", "So,
> > you want to be a developer?", and the "Developer FAQ" page, which
> > doesn't have to be more than removing outdated information and
> > refering to any (new) documentation on how to participate in the
> > PostgreSQL development and/or Commitfest workflow as a non-CFM.
>
> Agreed, a sweep of those materials would be helpful as well. I'm
> personally focused on CFM tasks, since it's fresh in my brain and
> documentation is almost non-existent for it, but if you have ideas for
> those areas, I definitely don't want to shut down that line of the
> conversation.

Nor would I want to hold you back on CFM documentation.

> > Additionally, we might want to add extra text to the "developers"
> > section of the main website [0] to refer to any new documentation.
> > This suggestion does depend on whether the new documentation has a
> > high value for potential community members.
>
> Right. So what kinds of info do we want to highlight in this
> documentation, to make it high-quality?

I think it would be a combined and abbreviated version of the detailed
manuals that we (will) have: The pages "Submitting a patch" and
"Reviewing a patch" on the wiki, and the CommitFest manual (plus
potentially info on CFBot).

The first part of "So, you want to be a developer?" seems like a very
good starting point for dense, high-quality entry-level documentation.
Each section should then further refer to the relevant sections of the
"Developer FAQ" and the "Submitting / Reviewing a Patch" pages for the
in-and-outs of the specific procedure (such as "installing development
dependencies", "reviewing changes", "code style", etc.).

> Drawing from some of the questions I've seen recently, we could talk about
> - CF "power" structure (perhaps simply to highlight that the CFM has
> no additional authority to get patches in)
> - the back-and-forth process on the mailing list, maybe including
> expected response times
> - what to do when a patch is returned (or rejected)
>
> > As an example, the GitHub mirror of the main PostgreSQL repository
> > receives a decent amount of pull request traffic. When a project has a
> > CONTRIBUTING.md -file at the top level people writing the pull request
> > message will be pointed to those contributing guidelines. This could
>
> (I think some text got cut here.)

... This could help reduce the amount of mis-addressed (maybe better
word: mis-located?) contributions, and potentially help the
contributor get involved at -hackers. Indeed this process is much more
involved than 'just' opening a pull request, but at least it is now
slightly more visible.

> The mirror bot will point you to the "So, you want to be a developer?"
> wiki when you open a PR, but I agree that a CONTRIBUTING doc would
> help prevent that small embarrassment.

That's news to me, but nice to see some improvements there. I have
previously noticed that there were PRs on GitHub that went unnoticed
for several weeks, so this bot is a significant improvement.


Kind regards,

Matthias van de Meent



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On Thu, Aug 4, 2022 at 3:00 PM Andres Freund <andres@anarazel.de> wrote:
> On 2022-08-04 11:19:28 -0700, Jacob Champion wrote:
> > My intention had not quite been for this to be a referendum on the
> > decision for every patch -- we can do that if it helps, but I don't
> > think we necessarily have to have unanimity on the bucketing for every
> > patch in order for the new state to be useful.
>
> Sorry, I should have been clearer. It wasn't mine either! I was just trying to
> understand what you see as the usecase / get a better feel for it. I'm now a
> bit more convinced it's useful than before.

Great!

> > >> - https://commitfest.postgresql.org/38/3310/
> > >
> > > I don't really understand why this has been RwF'd, doesn't seem that long
> > > since the last review leading to changes.
> >
> > Eight months without feedback, when we expect authors to turn around a
> > patch in two weeks or less to avoid being RwF'd, is a long time IMHO.
>
> Why is it better to mark it as lacks interested than RwF if there actually
> *has* been feedback?

Because I don't think the utility of RwF is in saying "we gave you
feedback once and then ghosted"; I think it's in saying "this patchset
needs work before the next round of review." If an author has
responded to the feedback and the patchset is just sitting there for
months, the existence of the feedback is less relevant.

> > I don't think a patch should sit motionless in CF for eight months; it's not
> > at all fair to the author.
>
> It's not great, I agree, but wishes don't conjure up resources :(

I see this less as a wish for resources, and more as an honest
admission -- we don't currently have enough resources to give each
patch the eyes it deserves, so if an author finds themselves in this
state, they'll have to put in some more work to find those eyes
somewhere.

> > >> - https://commitfest.postgresql.org/38/3050/
> > >
> > > Given that a non-author did a revision of the patch, listed a number of TODO
> > > items and said "I'll create regression tests firstly." - I don't think "lacks
> > > interest" would have been appropriate, and RwF is?
> >
> > That was six months ago, and prior to that there was another six month
> > silence. I'd say that lacks interest, and I don't feel like it's
> > currently reviewable in CF.
>
> I don't think the entry needs more review - it needs changes:
> https://www.postgresql.org/message-id/CAOKkKFtc45uNFoWYOCo4St19ayxrh-_%2B4TnZtwxGZz6-3k_GSA%40mail.gmail.com
> That contains quite a few things that should be changed.
>
> A patch that has gotten feedback, but that feedback hasn't been processed
> pretty much is the definition of RwF, no?

Looking through again, I see now what you're saying. Yes, I agree that
RwF would have been a fine fit there.

> I agree that "lacks interest" could be useful. But I'm wary of it becoming
> just a renaming if we end up marking patches that should be RwF or rejected as
> "lacks interest".

Agreed. This probably bleeds over into the other documentation thread
a bit -- how do we want to communicate the subtle points to people in
a CF?

--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Andres Freund
Date:
Hi,

On 2022-08-08 08:37:41 -0700, Jacob Champion wrote:
> Agreed. This probably bleeds over into the other documentation thread
> a bit -- how do we want to communicate the subtle points to people in
> a CF?

We should write a docs patch for it and then reference if from a bunch of
places. I started down that road a few years back [1] but unfortunately lost
steam.

Regards,

Andres

[1] https://postgr.es/m/20180302224056.3fps7kc6hokqk3th%40alap3.anarazel.de



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Bruce Momjian
Date:
On Wed, Aug  3, 2022 at 02:53:23PM -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > My impression is that a lot of the patches floating from CF to CF have gotten
> > sceptical feedback and at best a minor amount of work to address that has been
> > done.
> 
> That I think is a distinct issue: nobody wants to take on the
> unpleasant job of saying "no, we don't want this" in a final way.
> We may raise some objections but actually rejecting a patch is hard.
> So it tends to slide forward until the author gives up.

Agreed.  There is a sense when I look at patches in that status that
they seem like a good idea to someone and could be useful to someone,
but the overhead or complexity it would add to the software doesn't seem
warranted.  It is complex to explain that to someone, and since it is a
judgement call, not worth the argument.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson




Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On Mon, Aug 8, 2022 at 8:45 AM Andres Freund <andres@anarazel.de> wrote:
> On 2022-08-08 08:37:41 -0700, Jacob Champion wrote:
> > Agreed. This probably bleeds over into the other documentation thread
> > a bit -- how do we want to communicate the subtle points to people in
> > a CF?
>
> We should write a docs patch for it and then reference if from a bunch of
> places. I started down that road a few years back [1] but unfortunately lost
> steam.

As we approach a new CF, I'm reminded of this patch again.

Are there any concerns preventing a consensus here, that I can help
with? I can draft the docs patch that Andres has suggested, if that's
seen as a prerequisite.

Thanks,
--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
vignesh C
Date:
On Wed, 26 Oct 2022 at 04:25, Jacob Champion <jchampion@timescale.com> wrote:
>
> On Mon, Aug 8, 2022 at 8:45 AM Andres Freund <andres@anarazel.de> wrote:
> > On 2022-08-08 08:37:41 -0700, Jacob Champion wrote:
> > > Agreed. This probably bleeds over into the other documentation thread
> > > a bit -- how do we want to communicate the subtle points to people in
> > > a CF?
> >
> > We should write a docs patch for it and then reference if from a bunch of
> > places. I started down that road a few years back [1] but unfortunately lost
> > steam.
>
> As we approach a new CF, I'm reminded of this patch again.
>
> Are there any concerns preventing a consensus here, that I can help
> with? I can draft the docs patch that Andres has suggested, if that's
> seen as a prerequisite.

The patch does not apply on top of HEAD as in [1], please post a rebased patch:

=== Applying patches on top of PostgreSQL commit ID
e351f85418313e97c203c73181757a007dfda6d0 ===
=== applying patch ./0001-Add-a-Returned-Needs-more-interest-close-code.patch
patching file pgcommitfest/commitfest/migrations/0006_alter_patchoncommitfest_status.py
can't find file to patch at input line 57
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/pgcommitfest/commitfest/models.py
b/pgcommitfest/commitfest/models.py
|index 28722f0..433eb4a 100644
|--- a/pgcommitfest/commitfest/models.py
|+++ b/pgcommitfest/commitfest/models.py
--------------------------
No file to patch.  Skipping patch.
3 out of 3 hunks ignored
can't find file to patch at input line 85

[1] - http://cfbot.cputube.org/patch_41_3991.log

Regards,
Vignesh



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On Tue, Jan 3, 2023 at 4:14 AM vignesh C <vignesh21@gmail.com> wrote:
> The patch does not apply on top of HEAD as in [1], please post a rebased patch:

Hi Vignesh -- this is a patch for the CF app, not the Postgres repo,
so cfbot won't be able to apply it. Let me know if there's a better
place for me to put it.

Thanks,
--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
vignesh C
Date:
On Tue, 3 Jan 2023 at 22:01, Jacob Champion <jchampion@timescale.com> wrote:
>
> On Tue, Jan 3, 2023 at 4:14 AM vignesh C <vignesh21@gmail.com> wrote:
> > The patch does not apply on top of HEAD as in [1], please post a rebased patch:
>
> Hi Vignesh -- this is a patch for the CF app, not the Postgres repo,
> so cfbot won't be able to apply it. Let me know if there's a better
> place for me to put it.

I'm not sure if this should be included in commitfest as we generally
include the postgres repository patches in the commitfest. I felt we
could have the discussion in the thread and remove the entry from
commitfest.

Regards,
Vignesh



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On Tue, Jan 3, 2023 at 8:56 PM vignesh C <vignesh21@gmail.com> wrote:
> I'm not sure if this should be included in commitfest as we generally
> include the postgres repository patches in the commitfest. I felt we
> could have the discussion in the thread and remove the entry from
> commitfest.

Is there a good way to remind people that, hey, this exists as a
patchset? (Other than me pinging the list every so often.)

--Jacob



Re: [PATCH] CF app: add "Returned: Needs more interest"

From
Jacob Champion
Date:
On Wed, Jan 4, 2023 at 9:33 AM Jacob Champion <jchampion@timescale.com> wrote:
> Is there a good way to remind people that, hey, this exists as a
> patchset? (Other than me pinging the list every so often.)

I've withdrawn this patchset for now, but if anyone has any ideas on
where and how I can better propose features for CF itself, I'm all
ears.

Thanks,
--Jacob