| Message ID | 20161022113847.4001-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, Oct 22, 2016 at 01:38:47PM +0200, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > doc/patchwork | 9 +++++++++ > 1 file changed, 9 insertions(+) > create mode 100644 doc/patchwork > > diff --git a/doc/patchwork b/doc/patchwork > new file mode 100644 > index 0000000..9486e07 > --- /dev/null > +++ b/doc/patchwork > @@ -0,0 +1,9 @@ > +Patchwork states > + > +NEW: Initial state of new patches > +Accepted: The patch was pushed to the main master repository > +Rejected: The patch has been rejected > +Not Applicable: The patch does not apply to the main master repository > +Superseded: A newer version of the patch has been posted > +Changes Requested: The patch has been or is under review and changes have been requested > +RFC: The patch is not intended to be applied but only for comments no "dropped" or "invalid" state? (similar to a self rejected patch)
On Sat, Oct 22, 2016 at 02:12:18PM +0200, Clément Bœsch wrote: > On Sat, Oct 22, 2016 at 01:38:47PM +0200, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > doc/patchwork | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > create mode 100644 doc/patchwork > > > > diff --git a/doc/patchwork b/doc/patchwork > > new file mode 100644 > > index 0000000..9486e07 > > --- /dev/null > > +++ b/doc/patchwork > > @@ -0,0 +1,9 @@ > > +Patchwork states > > + > > +NEW: Initial state of new patches > > +Accepted: The patch was pushed to the main master repository > > +Rejected: The patch has been rejected > > +Not Applicable: The patch does not apply to the main master repository > > +Superseded: A newer version of the patch has been posted > > +Changes Requested: The patch has been or is under review and changes have been requested > > +RFC: The patch is not intended to be applied but only for comments > > no "dropped" or "invalid" state? (similar to a self rejected patch) Dropped state added anything else we need ? [...]
2016.10.22. 18:29 keltezéssel, Stephen Hutchinson írta: > On 10/22/2016 8:25 AM, Michael Niedermayer wrote: >> On Sat, Oct 22, 2016 at 02:12:18PM +0200, Clément Bœsch wrote: >>> On Sat, Oct 22, 2016 at 01:38:47PM +0200, Michael Niedermayer wrote: >>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>>> --- >>>> doc/patchwork | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> create mode 100644 doc/patchwork >>>> >>>> diff --git a/doc/patchwork b/doc/patchwork >>>> new file mode 100644 >>>> index 0000000..9486e07 >>>> --- /dev/null >>>> +++ b/doc/patchwork >>>> @@ -0,0 +1,9 @@ >>>> +Patchwork states >>>> + >>>> +NEW: Initial state of new patches >>>> +Accepted: The patch was pushed to the main master repository >>>> +Rejected: The patch has been rejected >>>> +Not Applicable: The patch does not apply to the main master >>>> repository >>>> +Superseded: A newer version of the patch has been posted >>>> +Changes Requested: The patch has been or is under review and >>>> changes have been requested >>>> +RFC: The patch is not intended to be applied but >>>> only for comments >>> >>> no "dropped" or "invalid" state? (similar to a self rejected patch) >> >> Dropped state added >> >> anything else we need ? >> >> [...] >> > > The other ones that appear in the list on Patchwork are > 'Under Review', 'Deferred', and 'Awaiting Upstream'. > > 'Under Review' is fairly self-explanatory, but when and why > a patch should be flagged that way (as opposed to simply > remaining as 'New' until it gets committed) isn't. > > 'Deferred' sounds like either holding off on commit to a > later date or kicking the can to somebody else, and... > > 'Awaiting Upstream' isn't all that clear about its purpose - > awaiting upstream for what? Review, commit, something else > I've not thought of? Is this the state that should be used > for patches that are queued up for commit? > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel under review: someone marked it because he/she investigate this patch. So the patch submitter and other potential reviewers may feel/be_sure that this patch is already handled by someone else. I suggest to use it. This is psychologic aspect that the patch submitter may feel more patienty toward review process opposite to mere "new" state. I think if we will use under_review state it will have an impact for community communication: if a patch remains in under_review state for long period the patch submitter may contact directly the specific person who put his/her patch into this state. But if a patch remains in new state for a longer period the submitter will ping the whole communitiy. I think the new->under_review is not a one-way change. The opposite direction under_review->new also should be available. I also suggest to communicate clearly the valid/expected state changes: new->(under_review|deferred|rejected|accepted|superseed|not_app) deferred->(new|under_review|deferred|rejected|accepted|superseed|not_app) under_review->(new||deferred|rejected|accepted|superseed|not_app) etc. bb
On Sat, Oct 22, 2016, at 09:49 AM, Bodecs Bela wrote: > > under review: someone marked it because he/she investigate this patch. > So the patch submitter and other potential reviewers may feel/be_sure > that this patch is already handled by someone else. > I suggest to use it. This is psychologic aspect that the patch submitter > may feel more patienty toward review process opposite to mere "new" > state. In my opinion that seems like unnecessary extra work. All patches in theory are to be "under review". Reviewing takes enough time, initiative, and motivation as is, and adding another, potentially superfluous step just over complicates it. Perhaps "Under Review" can just be removed, disabled, hidden, or documented as "this option is ignored". Same with "Deferred" and "Awaiting Upstream".
On Sat, Oct 22, 2016 at 10:41:29AM -0800, Lou Logan wrote: > On Sat, Oct 22, 2016, at 09:49 AM, Bodecs Bela wrote: > > > > under review: someone marked it because he/she investigate this patch. > > So the patch submitter and other potential reviewers may feel/be_sure > > that this patch is already handled by someone else. > > I suggest to use it. This is psychologic aspect that the patch submitter > > may feel more patienty toward review process opposite to mere "new" > > state. > > In my opinion that seems like unnecessary extra work. All patches in > theory are to be "under review". Reviewing takes enough time, > initiative, and motivation as is, and adding another, potentially > superfluous step just over complicates it. > > Perhaps "Under Review" can just be removed, disabled, hidden, or > documented as "this option is ignored". Same with "Deferred" and > "Awaiting Upstream". Removed "Under Review" and "Awaiting Upstream" Theres a patch with "Deferred" state, this would need to be changed first [...]
On Sat, Oct 22, 2016 at 09:20:20PM +0200, Michael Niedermayer wrote: > On Sat, Oct 22, 2016 at 10:41:29AM -0800, Lou Logan wrote: > > On Sat, Oct 22, 2016, at 09:49 AM, Bodecs Bela wrote: > > > > > > under review: someone marked it because he/she investigate this patch. > > > So the patch submitter and other potential reviewers may feel/be_sure > > > that this patch is already handled by someone else. > > > I suggest to use it. This is psychologic aspect that the patch submitter > > > may feel more patienty toward review process opposite to mere "new" > > > state. > > > > In my opinion that seems like unnecessary extra work. All patches in > > theory are to be "under review". Reviewing takes enough time, > > initiative, and motivation as is, and adding another, potentially > > superfluous step just over complicates it. > > > > Perhaps "Under Review" can just be removed, disabled, hidden, or > > documented as "this option is ignored". Same with "Deferred" and > > "Awaiting Upstream". > > Removed "Under Review" and "Awaiting Upstream" > Theres a patch with "Deferred" state, this would need to be changed > first patch applied [...]
diff --git a/doc/patchwork b/doc/patchwork new file mode 100644 index 0000000..9486e07 --- /dev/null +++ b/doc/patchwork @@ -0,0 +1,9 @@ +Patchwork states + +NEW: Initial state of new patches +Accepted: The patch was pushed to the main master repository +Rejected: The patch has been rejected +Not Applicable: The patch does not apply to the main master repository +Superseded: A newer version of the patch has been posted +Changes Requested: The patch has been or is under review and changes have been requested +RFC: The patch is not intended to be applied but only for comments
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- doc/patchwork | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 doc/patchwork