"rejected" vs "returned with feedback" in new CF app

On 4/7/15 3:33 PM, Tom Lane wrote:

I tried to mark the "UPDATE SET (*)" patch as "returned with feedback",
but the CF app informed me that if I did that the patch would
automatically be moved to the next commitfest. That seems completely
stupid. There is no need to reconsider it unless a new version of the
patch is forthcoming (which there may or may not ever be, but that's
beside the point for now). When and if the author does submit a new
patch, that would be the time to include it in the next commitfest, no?

I noticed that as well and have avoided closing some patches because of it.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Apr 7, 2015 at 3:35 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 4/7/15 3:33 PM, Tom Lane wrote:

I tried to mark the "UPDATE SET (*)" patch as "returned with feedback",
but the CF app informed me that if I did that the patch would
automatically be moved to the next commitfest. That seems completely
stupid. There is no need to reconsider it unless a new version of the
patch is forthcoming (which there may or may not ever be, but that's
beside the point for now). When and if the author does submit a new
patch, that would be the time to include it in the next commitfest, no?

I noticed that as well and have avoided closing some patches because of it.

Several people, including me, have complained about this before. I
hope that Magnus will fix it soon.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Apr 8, 2015 at 11:57 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Apr 7, 2015 at 3:35 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 4/7/15 3:33 PM, Tom Lane wrote:

I tried to mark the "UPDATE SET (*)" patch as "returned with feedback",
but the CF app informed me that if I did that the patch would
automatically be moved to the next commitfest. That seems completely
stupid. There is no need to reconsider it unless a new version of the
patch is forthcoming (which there may or may not ever be, but that's
beside the point for now). When and if the author does submit a new
patch, that would be the time to include it in the next commitfest, no?

I noticed that as well and have avoided closing some patches because of it.

Several people, including me, have complained about this before. I
hope that Magnus will fix it soon.

Yeah, you can find references about that here and there... And the
current behavior is confusing.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Apr 8, 2015 at 4:57 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Apr 7, 2015 at 3:35 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 4/7/15 3:33 PM, Tom Lane wrote:

I tried to mark the "UPDATE SET (*)" patch as "returned with feedback",
but the CF app informed me that if I did that the patch would
automatically be moved to the next commitfest. That seems completely
stupid. There is no need to reconsider it unless a new version of the
patch is forthcoming (which there may or may not ever be, but that's
beside the point for now). When and if the author does submit a new
patch, that would be the time to include it in the next commitfest, no?

I noticed that as well and have avoided closing some patches because of

it.

Several people, including me, have complained about this before. I
hope that Magnus will fix it soon.

Yeah, I think my doing so is more or less down to one of the hardest
problems in IT - naming things. As in, what should we call that level.

Right now we have "Committed", "Returned with feedback" and "Rejected" as
the statuses that indicates something is "done for this commitfest". I do
think we want to add another one of those to differentiate between these
two states -- we could flag it as just "returned with feedback" and not
move it, but if we do that we loose the ability to do statistics on it for
example, and in order to figure out what happened you have to go look at
the history details int he box at the bottom.

So i think we need a specific label for it. Any suggestions for what it
should be?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On April 8, 2015 9:28:50 PM GMT+02:00, Magnus Hagander <magnus@hagander.net> wrote:

On Wed, Apr 8, 2015 at 4:57 AM, Robert Haas <robertmhaas@gmail.com>
wrote:

On Tue, Apr 7, 2015 at 3:35 PM, Peter Eisentraut <peter_e@gmx.net>

wrote:

On 4/7/15 3:33 PM, Tom Lane wrote:

I tried to mark the "UPDATE SET (*)" patch as "returned with

feedback",

but the CF app informed me that if I did that the patch would
automatically be moved to the next commitfest. That seems

completely

stupid. There is no need to reconsider it unless a new version of

the

patch is forthcoming (which there may or may not ever be, but

that's

beside the point for now). When and if the author does submit a

new

patch, that would be the time to include it in the next

commitfest, no?

I noticed that as well and have avoided closing some patches

because of

it.

Several people, including me, have complained about this before. I
hope that Magnus will fix it soon.

Yeah, I think my doing so is more or less down to one of the hardest
problems in IT - naming things. As in, what should we call that level.

Right now we have "Committed", "Returned with feedback" and "Rejected"
as
the statuses that indicates something is "done for this commitfest". I
do
think we want to add another one of those to differentiate between
these
two states -- we could flag it as just "returned with feedback" and not
move it, but if we do that we loose the ability to do statistics on it
for
example, and in order to figure out what happened you have to go look
at
the history details int he box at the bottom.

So i think we need a specific label for it. Any suggestions for what it
should be?

I'm not convinced we really need a version that closes and moves a entry. But if we indeed want it we can just name it "moved".

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 04/08/2015 03:28 PM, Magnus Hagander wrote:

On Wed, Apr 8, 2015 at 4:57 AM, Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>> wrote:

On Tue, Apr 7, 2015 at 3:35 PM, Peter Eisentraut <peter_e@gmx.net
<mailto:peter_e@gmx.net>> wrote:

On 4/7/15 3:33 PM, Tom Lane wrote:

I tried to mark the "UPDATE SET (*)" patch as "returned with

feedback",

but the CF app informed me that if I did that the patch would
automatically be moved to the next commitfest. That seems

completely

stupid. There is no need to reconsider it unless a new version

of the

patch is forthcoming (which there may or may not ever be, but

that's

beside the point for now). When and if the author does submit

a new

patch, that would be the time to include it in the next

commitfest, no?

I noticed that as well and have avoided closing some patches

because of it.

Several people, including me, have complained about this before. I
hope that Magnus will fix it soon.

Yeah, I think my doing so is more or less down to one of the hardest
problems in IT - naming things. As in, what should we call that level.

Right now we have "Committed", "Returned with feedback" and "Rejected"
as the statuses that indicates something is "done for this
commitfest". I do think we want to add another one of those to
differentiate between these two states -- we could flag it as just
"returned with feedback" and not move it, but if we do that we loose
the ability to do statistics on it for example, and in order to figure
out what happened you have to go look at the history details int he
box at the bottom.

So i think we need a specific label for it. Any suggestions for what
it should be?

If we're moving it to the next commitfest, maybe "Delayed with
feedback". "Returned with feedback" should be putting the ball back in
the submitter's court for further action.

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Apr 7, 2015 at 12:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I tried to mark the "UPDATE SET (*)" patch as "returned with feedback",
but the CF app informed me that if I did that the patch would
automatically be moved to the next commitfest. That seems completely
stupid. There is no need to reconsider it unless a new version of the
patch is forthcoming (which there may or may not ever be, but that's
beside the point for now). When and if the author does submit a new
patch, that would be the time to include it in the next commitfest, no?

I ended up marking it "rejected" instead, but that seems a bit harsh.

While that is one possibility, given that it shows in the next CF as
"Waiting on Author"​, and lack of any other obvious place to put the CF
entry while it is in limbo, I'm not sure there is a problem here - though
I'm sure I and others can envision additional capabilities to make tracking
"committer" vs "author" responsibility easier.

I could see adding a "Moved to ToDo" status that denotes that we got tired
of "Waiting on Author" and decided to move the item to the ToDo list. The
same could be used to simply indicate that while the idea is solid the
current implementation is lacking. Quite a few ToDo items fall into that
category - and saying the patch is rejected while the concept is accepted
is valid feedback. Whether we want to distinguish between "Abandoned -
moved to ToDo" and "Unacceptable Implementation - moved to ToDo" is
something to consider once the idea of using the ToDo as a limbo area, in
addition to the next CF, is agreed upon.

Put another way, the logical conclusion to Tom's sentiment is to simply
remove everything "Waiting on Author" since there is never any guarantee
that a response will be forthcoming and then, if one is, the entry can be
added back into the current CF at that time. Leaving open items in the
prior CS doesn't seem to make sense and I do not know enough about the
application to determine how feasible it is to be a closed item from a
previous CFs back to life.

​David J.​

On Apr 9, 2015, at 1:08 AM, Andres Freund <andres@anarazel.de> wrote:

I'm not convinced we really need a version that closes and moves a entry. But if we indeed want it we can just name it "moved".

+1.

...Robert

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Apr 9, 2015 at 9:20 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Apr 9, 2015, at 1:08 AM, Andres Freund <andres@anarazel.de> wrote:

I'm not convinced we really need a version that closes and moves a entry. But if we indeed want it we can just name it "moved".

+1.

+1. Sounds like a good idea. It would be good to get something in this
area before the virtual deadline of 4/15, switching the current CF to
money time...
-- 
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Apr 9, 2015 2:20 AM, "Robert Haas" <robertmhaas@gmail.com> wrote:

On Apr 9, 2015, at 1:08 AM, Andres Freund <andres@anarazel.de> wrote:

I'm not convinced we really need a version that closes and moves a

entry. But if we indeed want it we can just name it "moved".

+1.

Is that at +1 for naming it moved, or for not having it? :-)

I can definitely go with moved. Buy I would like to keep it - the reason
for having it in the first place is to make the history of the patch follow
along when it goes to the next cf. If we don't have the move option, I
think it's likely that we'll be back to the same patch having multiple
completely unrelated entries in different cfs.

/Magnus

Magnus Hagander <magnus@hagander.net> writes:

On Apr 9, 2015 2:20 AM, "Robert Haas" <robertmhaas@gmail.com> wrote:

+1.

Is that at +1 for naming it moved, or for not having it? :-)

I can definitely go with moved. Buy I would like to keep it - the reason
for having it in the first place is to make the history of the patch follow
along when it goes to the next cf. If we don't have the move option, I
think it's likely that we'll be back to the same patch having multiple
completely unrelated entries in different cfs.

The problem with the whole thing is that you're asking the person doing
the "returned" marking to guess whether the patch will be resubmitted in
a future CF.

The right workflow here, IMO, is that a patch should be marked returned or
rejected, full stop; and then when/if the author submits a new version for
a future CF, there should be a way *at that time* to re-link the email
thread into that future CF.

"Moved" is really only applicable, I think, for cases where we punt a
patch to the next CF for lack of time.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Apr 9, 2015 at 2:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Apr 9, 2015 2:20 AM, "Robert Haas" <robertmhaas@gmail.com> wrote:

+1.

Is that at +1 for naming it moved, or for not having it? :-)

I can definitely go with moved. Buy I would like to keep it - the reason
for having it in the first place is to make the history of the patch

follow

along when it goes to the next cf. If we don't have the move option, I
think it's likely that we'll be back to the same patch having multiple
completely unrelated entries in different cfs.

The problem with the whole thing is that you're asking the person doing
the "returned" marking to guess whether the patch will be resubmitted in
a future CF.

The right workflow here, IMO, is that a patch should be marked returned or
rejected, full stop; and then when/if the author submits a new version for
a future CF, there should be a way *at that time* to re-link the email
thread into that future CF.

If we just link the email thread, that would mean we loose all those
precious annotations we just added support for. Is that really what you
meant? We also loose all history of a patch, and can't see that a previous
version existed in a previous commitfest, without manually checking each
and every one. But if that's a history we don't *want*, that's of course
doable, but it seems wrong to me?

I'm not necessarily saying that what we have now is right, but just giving
up on the history completely doesn't seem like a very good workflow to me.

We could always tell those people to "go back and find your old patch and
re-open it", but in fairness, are people likely to actually do that?

"Moved" is really only applicable, I think, for cases where we punt a

patch to the next CF for lack of time.

Well, that's basically what "returned with feedback" is now, so I guess
that one should just be renamed in that case. And we add a new "returned
with feedback" that closes out the patch and doesn't move it anywhere.
Which is pretty similar to the suggestion earlier in this thread except it
also swaps the two names.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On 2015-04-09 15:09:55 +0200, Magnus Hagander wrote:

If we just link the email thread, that would mean we loose all those
precious annotations we just added support for. Is that really what you
meant? We also loose all history of a patch, and can't see that a previous
version existed in a previous commitfest, without manually checking each
and every one. But if that's a history we don't *want*, that's of course
doable, but it seems wrong to me?

It'd be better if we kept them, but it's not *that* important imo. But
if the (documented) workflow would be to go to the old cf and click the
'move to next CF' button that'd not be a problem anyway.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Magnus Hagander wrote:

On Thu, Apr 9, 2015 at 2:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The right workflow here, IMO, is that a patch should be marked
returned or rejected, full stop; and then when/if the author submits
a new version for a future CF, there should be a way *at that time*
to re-link the email thread into that future CF.

If we just link the email thread, that would mean we loose all those
precious annotations we just added support for. Is that really what
you meant? We also loose all history of a patch, and can't see that a
previous version existed in a previous commitfest, without manually
checking each and every one. But if that's a history we don't *want*,
that's of course doable, but it seems wrong to me?

I'm not necessarily saying that what we have now is right, but just
giving up on the history completely doesn't seem like a very good
workflow to me.

We could always tell those people to "go back and find your old patch
and re-open it", but in fairness, are people likely to actually do
that?

I think it's convenient if the submitter can go to a previous commitfest
and set an RwF entry as again needing review in the open commitfest.
That would keep the CF-app-history intact. This should probably only be
allowed for patches that are either RwF or Rejected, and only in
commitfests that are already closed (perhaps allow it for the commitfest
in progress also?).

"Moved" is really only applicable, I think, for cases where we punt
a patch to the next CF for lack of time.

Well, that's basically what "returned with feedback" is now, so I
guess that one should just be renamed in that case.

Yes, keeping the current behavior with name "Moved to next CF" seems
good to me.

And we add a new "returned with feedback" that closes out the patch
and doesn't move it anywhere.

Sounds good.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 04/09/2015 09:09 AM, Magnus Hagander wrote:

"Moved" is really only applicable, I think, for cases where we punt a
patch to the next CF for lack of time.

Well, that's basically what "returned with feedback" is now, so I
guess that one should just be renamed in that case. And we add a new
"returned with feedback" that closes out the patch and doesn't move it
anywhere. Which is pretty similar to the suggestion earlier in this
thread except it also swaps the two names.

I think we should keep it, and see how it works in practice. I'd prefer
a name like "deferred" to "moved" - the latter is a workflow process
rather than a patch status, ISTM.

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thursday, April 9, 2015, Magnus Hagander <magnus@hagander.net> wrote:

On Thu, Apr 9, 2015 at 2:20 PM, Tom Lane <tgl@sss.pgh.pa.us
<javascript:_e(%7B%7D,'cvml','tgl@sss.pgh.pa.us');>> wrote:

Magnus Hagander <magnus@hagander.net
<javascript:_e(%7B%7D,'cvml','magnus@hagander.net');>> writes:

On Apr 9, 2015 2:20 AM, "Robert Haas" <robertmhaas@gmail.com

<javascript:_e(%7B%7D,'cvml','robertmhaas@gmail.com');>> wrote:

+1.

Is that at +1 for naming it moved, or for not having it? :-)

I can definitely go with moved. Buy I would like to keep it - the reason
for having it in the first place is to make the history of the patch

follow

along when it goes to the next cf. If we don't have the move option, I
think it's likely that we'll be back to the same patch having multiple
completely unrelated entries in different cfs.

The problem with the whole thing is that you're asking the person doing
the "returned" marking to guess whether the patch will be resubmitted in
a future CF.

The right workflow here, IMO, is that a patch should be marked returned or
rejected, full stop; and then when/if the author submits a new version for
a future CF, there should be a way *at that time* to re-link the email
thread into that future CF.

If we just link the email thread, that would mean we loose all those
precious annotations we just added support for. Is that really what you
meant? We also loose all history of a patch, and can't see that a previous
version existed in a previous commitfest, without manually checking each
and every one. But if that's a history we don't *want*, that's of course
doable, but it seems wrong to me?

I'm not necessarily saying that what we have now is right, but just giving
up on the history completely doesn't seem like a very good workflow to me.

We could always tell those people to "go back and find your old patch and
re-open it", but in fairness, are people likely to actually do that?

"Moved" is really only applicable, I think, for cases where we punt a

patch to the next CF for lack of time.

Well, that's basically what "returned with feedback" is now, so I guess
that one should just be renamed in that case. And we add a new "returned
with feedback" that closes out the patch and doesn't move it anywhere.
Which is pretty similar to the suggestion earlier in this thread except it
also swaps the two names.

Can we create a "fake" CF time period into which all of these "waiting on
author" entries can be placed and readily browsed/found instead of leaving
them in whatever CF they happened to stall in?

David J.

On Apr 9, 2015, at 5:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The problem with the whole thing is that you're asking the person doing
the "returned" marking to guess whether the patch will be resubmitted in
a future CF.

The right workflow here, IMO, is that a patch should be marked returned or
rejected, full stop; and then when/if the author submits a new version for
a future CF, there should be a way *at that time* to re-link the email
thread into that future CF.

Yeah, or a way to reactivate the old entry at that time. Any kind of routine carryover to the next CF is going to lead to an accumulation of dead patches with live CF entries; it should require some action to re-enter a previously returned patch in the latest CF.

...Robert

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

David G. Johnston wrote:

Can we create a "fake" CF time period into which all of these "waiting on
author" entries can be placed and readily browsed/found instead of leaving
them in whatever CF they happened to stall in?

This seems a good idea to me -- not a "fake CF", but a page listing all
the Returned With Feedback patches, regardless of which commitfest(s)
they're linked onto.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers