[PATCH net-next] docs: netdev: document patchwork patch states

[PATCH net-next] docs: netdev: document patchwork patch states

[Jakub Kicinski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=a, Size: 2627 bytes --]

The patchwork states are largely self-explanatory but small
ambiguities may still come up. Document how we interpret
the states in networking.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: corbet@lwn.net
CC: workflows@vger.kernel.org
CC: linux-doc@vger.kernel.org
---
 Documentation/process/maintainer-netdev.rst | 27 ++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
index c1c732e9748b..5d16fbb93d25 100644
--- a/Documentation/process/maintainer-netdev.rst
+++ b/Documentation/process/maintainer-netdev.rst
@@ -120,7 +120,32 @@ Status of a patch can be checked by looking at the main patchwork
   https://patchwork.kernel.org/project/netdevbpf/list/
 
 The "State" field will tell you exactly where things are at with your
-patch. Patches are indexed by the ``Message-ID`` header of the emails
+patch:
+
+================== =============================================================
+Patch state        Description
+================== =============================================================
+New, Under review  pending review, patch is in the maintainer’s queue for review
+Accepted           patch was applied to the appropriate networking tree, this is
+                   usually set automatically by the pw-bot
+Needs ACK          waiting for an ack from an area maintainer or testing
+Changes requested  patch has not passed the review, new revision is expected
+                   with appropriate code and commit message changes
+Rejected           patch has been rejected and new revision is not expected
+Not applicable     patch is expected to be applied outside of the networking
+                   subsystem
+Awaiting upstream  patch should be reviewed and handled by appropriate
+                   sub-maintainer, who will send it on to the networking trees
+Deferred           patch needs to be reposted later, usually due to dependency
+                   or because it was posted for a closed tree
+Superseded         new version of the patch was posted, usually set by the
+                   pw-bot
+RFC                not to be applied, usually not in maintainer’s review queue,
+                   pw-bot can automatically set patches to this state based
+		   on subject tags
+================== =============================================================
+
+Patches are indexed by the ``Message-ID`` header of the emails
 which carried them so if you have trouble finding your patch append
 the value of ``Message-ID`` to the URL above.
 
-- 
2.41.0

Re: [PATCH net-next] docs: netdev: document patchwork patch states

[Randy Dunlap]

Hi,

On 8/28/23 11:44, Jakub Kicinski wrote:
> The patchwork states are largely self-explanatory but small
> ambiguities may still come up. Document how we interpret
> the states in networking.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: corbet@lwn.net
> CC: workflows@vger.kernel.org
> CC: linux-doc@vger.kernel.org
> ---
>  Documentation/process/maintainer-netdev.rst | 27 ++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> index c1c732e9748b..5d16fbb93d25 100644
> --- a/Documentation/process/maintainer-netdev.rst
> +++ b/Documentation/process/maintainer-netdev.rst
> @@ -120,7 +120,32 @@ Status of a patch can be checked by looking at the main patchwork
>    https://patchwork.kernel.org/project/netdevbpf/list/
>  
>  The "State" field will tell you exactly where things are at with your
> -patch. Patches are indexed by the ``Message-ID`` header of the emails
> +patch:

                                                      of the patch's email.
?

> +
> +================== =============================================================
> +Patch state        Description
> +================== =============================================================
> +New, Under review  pending review, patch is in the maintainer’s queue for review
> +Accepted           patch was applied to the appropriate networking tree, this is
> +                   usually set automatically by the pw-bot
> +Needs ACK          waiting for an ack from an area maintainer or testing
> +Changes requested  patch has not passed the review, new revision is expected
> +                   with appropriate code and commit message changes
> +Rejected           patch has been rejected and new revision is not expected
> +Not applicable     patch is expected to be applied outside of the networking
> +                   subsystem
> +Awaiting upstream  patch should be reviewed and handled by appropriate
> +                   sub-maintainer, who will send it on to the networking trees
> +Deferred           patch needs to be reposted later, usually due to dependency
> +                   or because it was posted for a closed tree
> +Superseded         new version of the patch was posted, usually set by the
> +                   pw-bot
> +RFC                not to be applied, usually not in maintainer’s review queue,
> +                   pw-bot can automatically set patches to this state based
> +		   on subject tags

Nit:
Above line uses tabs for indentation. All other lines here use spaces.

> +================== =============================================================
> +
> +Patches are indexed by the ``Message-ID`` header of the emails
>  which carried them so if you have trouble finding your patch append
>  the value of ``Message-ID`` to the URL above.
>  

Thanks.
-- 
~Randy

Re: [PATCH net-next] docs: netdev: document patchwork patch states

[Laurent Pinchart]

Hi Jakub,

Thank you for the patch.

On Mon, Aug 28, 2023 at 11:44:41AM -0700, Jakub Kicinski wrote:
> The patchwork states are largely self-explanatory but small
> ambiguities may still come up. Document how we interpret
> the states in networking.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: corbet@lwn.net
> CC: workflows@vger.kernel.org
> CC: linux-doc@vger.kernel.org
> ---
>  Documentation/process/maintainer-netdev.rst | 27 ++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst
> index c1c732e9748b..5d16fbb93d25 100644
> --- a/Documentation/process/maintainer-netdev.rst
> +++ b/Documentation/process/maintainer-netdev.rst
> @@ -120,7 +120,32 @@ Status of a patch can be checked by looking at the main patchwork
>    https://patchwork.kernel.org/project/netdevbpf/list/
>  
>  The "State" field will tell you exactly where things are at with your
> -patch. Patches are indexed by the ``Message-ID`` header of the emails
> +patch:
> +
> +================== =============================================================
> +Patch state        Description
> +================== =============================================================
> +New, Under review  pending review, patch is in the maintainer’s queue for review

Is there a meaningful distinction between "New" and "Under review", or
are they exactly the same ? The former sounds like nobody has looked at
the patch yet, while the latter seems to indicate someone has assigned
the task of reviewing the patch to themselves, but maybe netdev uses
those two states differently ?

> +Accepted           patch was applied to the appropriate networking tree, this is
> +                   usually set automatically by the pw-bot
> +Needs ACK          waiting for an ack from an area maintainer or testing

How does this differ from "Under review" ?

> +Changes requested  patch has not passed the review, new revision is expected
> +                   with appropriate code and commit message changes
> +Rejected           patch has been rejected and new revision is not expected
> +Not applicable     patch is expected to be applied outside of the networking
> +                   subsystem
> +Awaiting upstream  patch should be reviewed and handled by appropriate
> +                   sub-maintainer, who will send it on to the networking trees
> +Deferred           patch needs to be reposted later, usually due to dependency
> +                   or because it was posted for a closed tree
> +Superseded         new version of the patch was posted, usually set by the
> +                   pw-bot
> +RFC                not to be applied, usually not in maintainer’s review queue,
> +                   pw-bot can automatically set patches to this state based
> +		   on subject tags

Incorrect indentation.

> +================== =============================================================
> +
> +Patches are indexed by the ``Message-ID`` header of the emails
>  which carried them so if you have trouble finding your patch append
>  the value of ``Message-ID`` to the URL above.
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH net-next] docs: netdev: document patchwork patch states

[Jakub Kicinski]

On Mon, 28 Aug 2023 12:05:45 -0700 Randy Dunlap wrote:
> >  The "State" field will tell you exactly where things are at with your
> > -patch. Patches are indexed by the ``Message-ID`` header of the emails
> > +patch:  
> 
>                                                       of the patch's email.

The diff may be a little confusing. The full sentence is below..

> > +================== =============================================================
> > +Patch state        Description
> > +================== =============================================================
> > +New, Under review  pending review, patch is in the maintainer’s queue for review
> > +Accepted           patch was applied to the appropriate networking tree, this is
> > +                   usually set automatically by the pw-bot
> > +Needs ACK          waiting for an ack from an area maintainer or testing
> > +Changes requested  patch has not passed the review, new revision is expected
> > +                   with appropriate code and commit message changes
> > +Rejected           patch has been rejected and new revision is not expected
> > +Not applicable     patch is expected to be applied outside of the networking
> > +                   subsystem
> > +Awaiting upstream  patch should be reviewed and handled by appropriate
> > +                   sub-maintainer, who will send it on to the networking trees
> > +Deferred           patch needs to be reposted later, usually due to dependency
> > +                   or because it was posted for a closed tree
> > +Superseded         new version of the patch was posted, usually set by the
> > +                   pw-bot
> > +RFC                not to be applied, usually not in maintainer’s review queue,
> > +                   pw-bot can automatically set patches to this state based
> > +		   on subject tags  
> 
> Nit:
> Above line uses tabs for indentation. All other lines here use spaces.

Thanks, will fix.

> > +================== =============================================================
> > +
> > +Patches are indexed by the ``Message-ID`` header of the emails
> >  which carried them so if you have trouble finding your patch append

..here.

Re: [PATCH net-next] docs: netdev: document patchwork patch states

[Randy Dunlap]

On 8/28/23 12:17, Jakub Kicinski wrote:
> On Mon, 28 Aug 2023 12:05:45 -0700 Randy Dunlap wrote:
>>>  The "State" field will tell you exactly where things are at with your
>>> -patch. Patches are indexed by the ``Message-ID`` header of the emails
>>> +patch:  
>>
>>                                                       of the patch's email.
> 
> The diff may be a little confusing. The full sentence is below..
> 


Ah yes, my bad. Thanks.

>>> +================== =============================================================
>>> +Patch state        Description
>>> +================== =============================================================
>>> +New, Under review  pending review, patch is in the maintainer’s queue for review
>>> +Accepted           patch was applied to the appropriate networking tree, this is
>>> +                   usually set automatically by the pw-bot
>>> +Needs ACK          waiting for an ack from an area maintainer or testing
>>> +Changes requested  patch has not passed the review, new revision is expected
>>> +                   with appropriate code and commit message changes
>>> +Rejected           patch has been rejected and new revision is not expected
>>> +Not applicable     patch is expected to be applied outside of the networking
>>> +                   subsystem
>>> +Awaiting upstream  patch should be reviewed and handled by appropriate
>>> +                   sub-maintainer, who will send it on to the networking trees
>>> +Deferred           patch needs to be reposted later, usually due to dependency
>>> +                   or because it was posted for a closed tree
>>> +Superseded         new version of the patch was posted, usually set by the
>>> +                   pw-bot
>>> +RFC                not to be applied, usually not in maintainer’s review queue,
>>> +                   pw-bot can automatically set patches to this state based
>>> +		   on subject tags  
>>
>> Nit:
>> Above line uses tabs for indentation. All other lines here use spaces.
> 
> Thanks, will fix.
> 
>>> +================== =============================================================
>>> +
>>> +Patches are indexed by the ``Message-ID`` header of the emails
>>>  which carried them so if you have trouble finding your patch append
> 
> ..here.

-- 
~Randy

Re: [PATCH net-next] docs: netdev: document patchwork patch states

[Jakub Kicinski]

On Mon, 28 Aug 2023 22:09:11 +0300 Laurent Pinchart wrote:
> > +New, Under review  pending review, patch is in the maintainer’s queue for review  
> 
> Is there a meaningful distinction between "New" and "Under review", or
> are they exactly the same ? The former sounds like nobody has looked at
> the patch yet, while the latter seems to indicate someone has assigned
> the task of reviewing the patch to themselves, but maybe netdev uses
> those two states differently ?

The honest answer is that I don't know. I used to think that the
distinction is as you described - after someone done the initial triage
on the patch it goes New -> Under review.

But there's little consistency with that happening and it's unclear what
"initial triage" constitutes at this stage, so for all practical
purposes New == Under review.

> > +Accepted           patch was applied to the appropriate networking tree, this is
> > +                   usually set automatically by the pw-bot
> > +Needs ACK          waiting for an ack from an area maintainer or testing  
> 
> How does this differ from "Under review" ?

This indicates that netdev maintainers are waiting for someone who
is not a netdev maintainer. Let me s/maintainer/expert/ to make that
clearer-ish.