diff mbox series

[RFC,2/3] MAINTAINERS, Handbook: Subsystem Profile

Message ID 154225760492.2499188.14152986544451112930.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive)
State New, archived
Headers show
Series Maintainer Handbook: Subsystem Profile | expand

Commit Message

Dan Williams Nov. 15, 2018, 4:53 a.m. UTC
As presented at the 2018 Linux Plumbers conference [1], the Subsystem
Profile is proposed as a way to reduce friction between committers and
maintainers and perhaps encourage conversations amongst maintainers
about best practice policies.

The profile contains short answers to some of the common policy
questions a contributor might have, or that a maintainer might consider
formalizing. The current list of maintenance policies is:

Overview: General introduction to maintaining the subsystem
Core: List of source files considered core
Leaf: List of source files that consume core functionality
Patches or Pull requests: Simple statement of expected submission format
Last -rc for new feature submissions: Expected lead time for submissions
Last -rc to merge features: Deadline for merge decisions
Non-author Ack / Review Tags Required: Patch review economics
Test Suite: Pass this suite before requesting inclusion
Resubmit Cadence: When to ping the maintainer
Trusted Reviewers: Help for triaging patches
Time Zone / Office Hours: When might a maintainer be available
Checkpatch / Style Cleanups: Policy on pure cleanup patches
Off-list review: Request for review gates
TODO: Potential development tasks up for grabs, or active focus areas

The goal of the Subsystem Profile is to set expectations for
contributors and interim or replacement maintainers for a subsystem.

See Documentation/maintainer/subsystem-profile.rst for more details, and
a follow-on example profile for the libnvdimm subsystem.

[1]: https://linuxplumbersconf.org/event/2/contributions/59/

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Steve French <stfrench@microsoft.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tobin C. Harding <me@tobin.cc>
Cc: Olof Johansson <olof@lixom.net>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joe Perches <joe@perches.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/maintainer/index.rst             |    1 
 Documentation/maintainer/subsystem-profile.rst |  145 ++++++++++++++++++++++++
 MAINTAINERS                                    |    4 +
 3 files changed, 150 insertions(+)
 create mode 100644 Documentation/maintainer/subsystem-profile.rst

Comments

Julia Lawall Nov. 15, 2018, 5:48 a.m. UTC | #1
On Wed, 14 Nov 2018, Dan Williams wrote:

> As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> Profile is proposed as a way to reduce friction between committers and
> maintainers and perhaps encourage conversations amongst maintainers
> about best practice policies.
>
> The profile contains short answers to some of the common policy
> questions a contributor might have, or that a maintainer might consider
> formalizing. The current list of maintenance policies is:
>
> Overview: General introduction to maintaining the subsystem
> Core: List of source files considered core
> Leaf: List of source files that consume core functionality
> Patches or Pull requests: Simple statement of expected submission format
> Last -rc for new feature submissions: Expected lead time for submissions
> Last -rc to merge features: Deadline for merge decisions
> Non-author Ack / Review Tags Required: Patch review economics
> Test Suite: Pass this suite before requesting inclusion
> Resubmit Cadence: When to ping the maintainer
> Trusted Reviewers: Help for triaging patches
> Time Zone / Office Hours: When might a maintainer be available
> Checkpatch / Style Cleanups: Policy on pure cleanup patches
> Off-list review: Request for review gates
> TODO: Potential development tasks up for grabs, or active focus areas

How about patch subject lines?  What is the formula that should be used to
transform the name(s) of the affected file(s) into an appropriate suject
line?

thanks,
julia


>
> The goal of the Subsystem Profile is to set expectations for
> contributors and interim or replacement maintainers for a subsystem.
>
> See Documentation/maintainer/subsystem-profile.rst for more details, and
> a follow-on example profile for the libnvdimm subsystem.
>
> [1]: https://linuxplumbersconf.org/event/2/contributions/59/
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Steve French <stfrench@microsoft.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tobin C. Harding <me@tobin.cc>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/maintainer/index.rst             |    1
>  Documentation/maintainer/subsystem-profile.rst |  145 ++++++++++++++++++++++++
>  MAINTAINERS                                    |    4 +
>  3 files changed, 150 insertions(+)
>  create mode 100644 Documentation/maintainer/subsystem-profile.rst
>
> diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst
> index 2a14916930cb..1e6b1aaa6024 100644
> --- a/Documentation/maintainer/index.rst
> +++ b/Documentation/maintainer/index.rst
> @@ -11,4 +11,5 @@ additions to this manual.
>
>     configure-git
>     pull-requests
> +   subsystem-profile
>
> diff --git a/Documentation/maintainer/subsystem-profile.rst b/Documentation/maintainer/subsystem-profile.rst
> new file mode 100644
> index 000000000000..a74b624e0972
> --- /dev/null
> +++ b/Documentation/maintainer/subsystem-profile.rst
> @@ -0,0 +1,145 @@
> +.. _subsystemprofile:
> +
> +Subsystem Profile
> +=================
> +
> +The Subsystem Profile is a collection of policy positions that a
> +maintainer or maintainer team establishes for the their subsystem. While
> +there is a wide range of technical nuance on maintaining disparate
> +sections of the kernel, the Subsystem Profile documents a known set of
> +major process policies that vary between subsystems.  What follows is a
> +list of policy questions a maintainer can answer and include a document
> +in the kernel, or on an external website. It advertises to other
> +maintainers and contributors the local policy of the subsystem. Some
> +sections are optional like "Overview", "Off-list review", and "TODO".
> +The others are recommended for all subsystems to address, but no section
> +is mandatory. In addition there are no wrong answers, just document how
> +a subsystem typically operates. Note that the profile follows the
> +subsystem not the maintainer, i.e. there is no expectation that a
> +maintainer of multiple subsystems deploys the same policy across those
> +subsystems.
> +
> +
> +Overview
> +--------
> +In this optional section of the profile provide a free form overview of
> +the subsystem written as a hand-off document. In other words write a
> +note to someone that would receive the “keys to the castle” in the event
> +of extended or unexpected absence. “So, you have recently become the
> +maintainer of the XYZ subsystem, condolences, it is a thankless job,
> +here is the lay of the land.” Details to consider are the extended
> +details that are not included in MAINTAINERS, and not addressed by the
> +other profile questions below. For example details like, who has access
> +to the git tree, branches that are pulled into -next, relevant
> +specifications, issue trackers, and sensitive code areas. If available
> +the Overview should link to other subsystem documentation that may
> +clarify, re-iterate, emphasize / de-emphasize portions of the global
> +process documentation for contributors (CodingStyle, SubmittingPatches,
> +etc...).
> +
> +
> +Core
> +----
> +A list of F: tags (as described by MAINTAINERS) listing what the
> +maintainer considers to be core files. The review and lead time
> +constraints for 'core' code may be stricter given the increased
> +sensitivity and risk of change.
> +
> +
> +Patches or Pull requests
> +------------------------
> +Some subsystems allow contributors to send pull requests, most require
> +mailed patches. State “Patches only”, or “Pull requests accepted”.
> +
> +
> +Last -rc for new feature submissions
> +------------------------------------
> +New feature submissions targeting the next merge window should have
> +their first posting for consideration before this point. Patches that
> +are submitted after this point should be clear that they are targeting
> +the NEXT+1 merge window, or should come with sufficient justification
> +why they should be considered on an expedited schedule. A general
> +guideline is to set expectation with contributors that new feature
> +submissions should appear before -rc5. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.
> +
> +
> +Last -rc to merge features
> +--------------------------
> +Indicate to contributors the point at which an as yet un-applied patch
> +set will need to wait for the NEXT+1 merge window. Of course there is no
> +obligation to ever except any given patchset, but if the review has not
> +concluded by this point the expectation the contributor should wait and
> +resubmit for the following merge window. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.
> +
> +
> +Non-author Ack / Review Tags Required
> +-------------------------------------
> +Let contributors and other maintainers know whether they can expect to
> +see the maintainer self-commit patches without 3rd-party review. Some
> +subsystem developer communities are so small as to make this requirement
> +impractical. Others may have been bootstrapped by a submission of
> +self-reviewed code at the outset, but have since moved to a
> +non-author review-required stance. This section sets expectations on the
> +code-review economics in the subsystem. For example, can a contributor
> +trade review of a maintainer's, or other contributor's patches in
> +exchange for consideration of their own.
> +
> +
> +Test Suite
> +----------
> +Indicate the test suite all patches are expected to pass before being
> +submitted for inclusion consideration.
> +
> +
> +Resubmit Cadence
> +----------------
> +Define a rate at which a contributor should wait to resubmit a patchset
> +that has not yet received comments. A general guideline is to try to
> +meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
> +a patch set.
> +
> +
> +Trusted Reviewers
> +-----------------
> +While a maintainer / maintainer-team is expected to be reviewer of last
> +resort the review load is less onerous when distributed amongst
> +contributors and or a trusted set of individuals.  This section is
> +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> +reviewers that should always be copied on a patch submission, the
> +trusted reviewers here are individuals contributors can reach out to if
> +a few 'Resubmit Cadence' intervals have gone by without maintainer
> +action, or to otherwise consult for advice.
> +
> +
> +Time Zone / Office Hours
> +------------------------
> +Let contributors know the time of day when one or more maintainers are
> +usually actively monitoring the mailing list.
> +
> +
> +Checkpatch / Style Cleanups
> +---------------------------
> +For subsystems with long standing code bases it is reasonable to decline
> +to accept pure coding-style fixup patches. This is where you can let
> +contributors know “Standalone style-cleanups are welcome”,
> +“Style-cleanups to existing code only welcome with other feature
> +changes”, or “Standalone style-cleanups to existing code are not
> +welcome”.
> +
> +
> +Off-list review
> +---------------
> +A maintainer may optionally require that contributors seek prior review
> +of patches before initial submission for upstream. For example,
> +“Developers from organization X, please seek internal review before
> +requesting upstream review”. This policy identifies occasions where a
> +maintainer wants to reflect some of the review load back to an
> +organization.
> +
> +
> +TODO
> +----
> +In this optional section include a list of work items that might be
> +suitable for onboarding a new developer to the subsystem.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83b7b3943a12..bb4a83a7684d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -99,6 +99,10 @@ Descriptions of section entries:
>  	   Obsolete:	Old code. Something tagged obsolete generally means
>  			it has been replaced by a better system and you
>  			should be using that.
> +	P: Subsystem Profile document for the maintainer entry.  This
> +	   is either an in-tree file or a URI to a document.  The
> +	   contents of a Subsystem Profile are described in
> +	   Documentation/maintainer/subsystem-profile.rst.
>  	F: Files and directories with wildcard patterns.
>  	   A trailing slash includes all files and subdirectory files.
>  	   F:	drivers/net/	all files in and below drivers/net
>
> _______________________________________________
> Ksummit-discuss mailing list
> Ksummit-discuss@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
>
Mauro Carvalho Chehab Nov. 15, 2018, 5:49 a.m. UTC | #2
Em Wed, 14 Nov 2018 20:53:25 -0800
Dan Williams <dan.j.williams@intel.com> escreveu:

> As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> Profile is proposed as a way to reduce friction between committers and
> maintainers and perhaps encourage conversations amongst maintainers
> about best practice policies.
> 
> The profile contains short answers to some of the common policy
> questions a contributor might have, or that a maintainer might consider
> formalizing. The current list of maintenance policies is:
> 
> Overview: General introduction to maintaining the subsystem
> Core: List of source files considered core
> Leaf: List of source files that consume core functionality
> Patches or Pull requests: Simple statement of expected submission format
> Last -rc for new feature submissions: Expected lead time for submissions
> Last -rc to merge features: Deadline for merge decisions
> Non-author Ack / Review Tags Required: Patch review economics
> Test Suite: Pass this suite before requesting inclusion
> Resubmit Cadence: When to ping the maintainer

> Trusted Reviewers: Help for triaging patches

There is one detail with regards to reviewing process that I'm not sure
how to express. There are some subsystems with co-maintainers, in the
sense that all co-maintainers are equally responsible for the subsystem.

There are other cases where there are sub-maintainers. That's, for example,
the model we use on media. There, we have different sub-maintainers for:

	- V4L2 drivers
	- Camera Sensors
	- Remote Controllers
	- HDMI CEC
	- DVB
	- Media Controller

The usual workflow is that they work as both reviewers and committers.
After they commit a certain amount of patches, they submit for me to
do a final review. This way, most of media patches have at least two
SOBs from non-authors.

On this model, a sub-maintainer is more than a trusted reviewer. Not sure
how to reflect it on this template.

I'll do a better review of the profile when I'll try to write a subsystem
profile for media.

Regards,
Mauro

> Time Zone / Office Hours: When might a maintainer be available
> Checkpatch / Style Cleanups: Policy on pure cleanup patches
> Off-list review: Request for review gates
> TODO: Potential development tasks up for grabs, or active focus areas
> 
> The goal of the Subsystem Profile is to set expectations for
> contributors and interim or replacement maintainers for a subsystem.
> 
> See Documentation/maintainer/subsystem-profile.rst for more details, and
> a follow-on example profile for the libnvdimm subsystem.
> 
> [1]: https://linuxplumbersconf.org/event/2/contributions/59/
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Steve French <stfrench@microsoft.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tobin C. Harding <me@tobin.cc>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/maintainer/index.rst             |    1 
>  Documentation/maintainer/subsystem-profile.rst |  145 ++++++++++++++++++++++++
>  MAINTAINERS                                    |    4 +
>  3 files changed, 150 insertions(+)
>  create mode 100644 Documentation/maintainer/subsystem-profile.rst
> 
> diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst
> index 2a14916930cb..1e6b1aaa6024 100644
> --- a/Documentation/maintainer/index.rst
> +++ b/Documentation/maintainer/index.rst
> @@ -11,4 +11,5 @@ additions to this manual.
>  
>     configure-git
>     pull-requests
> +   subsystem-profile
>  
> diff --git a/Documentation/maintainer/subsystem-profile.rst b/Documentation/maintainer/subsystem-profile.rst
> new file mode 100644
> index 000000000000..a74b624e0972
> --- /dev/null
> +++ b/Documentation/maintainer/subsystem-profile.rst
> @@ -0,0 +1,145 @@
> +.. _subsystemprofile:
> +
> +Subsystem Profile
> +=================
> +
> +The Subsystem Profile is a collection of policy positions that a
> +maintainer or maintainer team establishes for the their subsystem. While
> +there is a wide range of technical nuance on maintaining disparate
> +sections of the kernel, the Subsystem Profile documents a known set of
> +major process policies that vary between subsystems.  What follows is a
> +list of policy questions a maintainer can answer and include a document
> +in the kernel, or on an external website. It advertises to other
> +maintainers and contributors the local policy of the subsystem. Some
> +sections are optional like "Overview", "Off-list review", and "TODO".
> +The others are recommended for all subsystems to address, but no section
> +is mandatory. In addition there are no wrong answers, just document how
> +a subsystem typically operates. Note that the profile follows the
> +subsystem not the maintainer, i.e. there is no expectation that a
> +maintainer of multiple subsystems deploys the same policy across those
> +subsystems.
> +
> +
> +Overview
> +--------
> +In this optional section of the profile provide a free form overview of
> +the subsystem written as a hand-off document. In other words write a
> +note to someone that would receive the “keys to the castle” in the event
> +of extended or unexpected absence. “So, you have recently become the
> +maintainer of the XYZ subsystem, condolences, it is a thankless job,
> +here is the lay of the land.” Details to consider are the extended
> +details that are not included in MAINTAINERS, and not addressed by the
> +other profile questions below. For example details like, who has access
> +to the git tree, branches that are pulled into -next, relevant
> +specifications, issue trackers, and sensitive code areas. If available
> +the Overview should link to other subsystem documentation that may
> +clarify, re-iterate, emphasize / de-emphasize portions of the global
> +process documentation for contributors (CodingStyle, SubmittingPatches,
> +etc...).
> +
> +
> +Core
> +----
> +A list of F: tags (as described by MAINTAINERS) listing what the
> +maintainer considers to be core files. The review and lead time
> +constraints for 'core' code may be stricter given the increased
> +sensitivity and risk of change.
> +
> +
> +Patches or Pull requests
> +------------------------
> +Some subsystems allow contributors to send pull requests, most require
> +mailed patches. State “Patches only”, or “Pull requests accepted”.
> +
> +
> +Last -rc for new feature submissions
> +------------------------------------
> +New feature submissions targeting the next merge window should have
> +their first posting for consideration before this point. Patches that
> +are submitted after this point should be clear that they are targeting
> +the NEXT+1 merge window, or should come with sufficient justification
> +why they should be considered on an expedited schedule. A general
> +guideline is to set expectation with contributors that new feature
> +submissions should appear before -rc5. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.
> +
> +
> +Last -rc to merge features
> +--------------------------
> +Indicate to contributors the point at which an as yet un-applied patch
> +set will need to wait for the NEXT+1 merge window. Of course there is no
> +obligation to ever except any given patchset, but if the review has not
> +concluded by this point the expectation the contributor should wait and
> +resubmit for the following merge window. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.
> +
> +
> +Non-author Ack / Review Tags Required
> +-------------------------------------
> +Let contributors and other maintainers know whether they can expect to
> +see the maintainer self-commit patches without 3rd-party review. Some
> +subsystem developer communities are so small as to make this requirement
> +impractical. Others may have been bootstrapped by a submission of
> +self-reviewed code at the outset, but have since moved to a
> +non-author review-required stance. This section sets expectations on the
> +code-review economics in the subsystem. For example, can a contributor
> +trade review of a maintainer's, or other contributor's patches in
> +exchange for consideration of their own.
> +
> +
> +Test Suite
> +----------
> +Indicate the test suite all patches are expected to pass before being
> +submitted for inclusion consideration.
> +
> +
> +Resubmit Cadence
> +----------------
> +Define a rate at which a contributor should wait to resubmit a patchset
> +that has not yet received comments. A general guideline is to try to
> +meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
> +a patch set.
> +
> +
> +Trusted Reviewers
> +-----------------
> +While a maintainer / maintainer-team is expected to be reviewer of last
> +resort the review load is less onerous when distributed amongst
> +contributors and or a trusted set of individuals.  This section is
> +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> +reviewers that should always be copied on a patch submission, the
> +trusted reviewers here are individuals contributors can reach out to if
> +a few 'Resubmit Cadence' intervals have gone by without maintainer
> +action, or to otherwise consult for advice.
> +
> +
> +Time Zone / Office Hours
> +------------------------
> +Let contributors know the time of day when one or more maintainers are
> +usually actively monitoring the mailing list.
> +
> +
> +Checkpatch / Style Cleanups
> +---------------------------
> +For subsystems with long standing code bases it is reasonable to decline
> +to accept pure coding-style fixup patches. This is where you can let
> +contributors know “Standalone style-cleanups are welcome”,
> +“Style-cleanups to existing code only welcome with other feature
> +changes”, or “Standalone style-cleanups to existing code are not
> +welcome”.
> +
> +
> +Off-list review
> +---------------
> +A maintainer may optionally require that contributors seek prior review
> +of patches before initial submission for upstream. For example,
> +“Developers from organization X, please seek internal review before
> +requesting upstream review”. This policy identifies occasions where a
> +maintainer wants to reflect some of the review load back to an
> +organization.
> +
> +
> +TODO
> +----
> +In this optional section include a list of work items that might be
> +suitable for onboarding a new developer to the subsystem.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83b7b3943a12..bb4a83a7684d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -99,6 +99,10 @@ Descriptions of section entries:
>  	   Obsolete:	Old code. Something tagged obsolete generally means
>  			it has been replaced by a better system and you
>  			should be using that.
> +	P: Subsystem Profile document for the maintainer entry.  This
> +	   is either an in-tree file or a URI to a document.  The
> +	   contents of a Subsystem Profile are described in
> +	   Documentation/maintainer/subsystem-profile.rst.
>  	F: Files and directories with wildcard patterns.
>  	   A trailing slash includes all files and subdirectory files.
>  	   F:	drivers/net/	all files in and below drivers/net
> 
> _______________________________________________
> Ksummit-discuss mailing list
> Ksummit-discuss@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss




Cheers,
Mauro
Geert Uytterhoeven Nov. 15, 2018, 7:58 a.m. UTC | #3
Hi Dan,

On Thu, Nov 15, 2018 at 6:05 AM Dan Williams <dan.j.williams@intel.com> wrote:
> As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> Profile is proposed as a way to reduce friction between committers and
> maintainers and perhaps encourage conversations amongst maintainers
> about best practice policies.
>
> The profile contains short answers to some of the common policy
> questions a contributor might have, or that a maintainer might consider
> formalizing. The current list of maintenance policies is:
>
> Overview: General introduction to maintaining the subsystem
> Core: List of source files considered core
> Leaf: List of source files that consume core functionality
> Patches or Pull requests: Simple statement of expected submission format
> Last -rc for new feature submissions: Expected lead time for submissions
> Last -rc to merge features: Deadline for merge decisions
> Non-author Ack / Review Tags Required: Patch review economics
> Test Suite: Pass this suite before requesting inclusion
> Resubmit Cadence: When to ping the maintainer
> Trusted Reviewers: Help for triaging patches
> Time Zone / Office Hours: When might a maintainer be available
> Checkpatch / Style Cleanups: Policy on pure cleanup patches
> Off-list review: Request for review gates
> TODO: Potential development tasks up for grabs, or active focus areas
>
> The goal of the Subsystem Profile is to set expectations for
> contributors and interim or replacement maintainers for a subsystem.
>
> See Documentation/maintainer/subsystem-profile.rst for more details, and
> a follow-on example profile for the libnvdimm subsystem.

> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/maintainer/subsystem-profile.rst

> +Last -rc to merge features
> +--------------------------
> +Indicate to contributors the point at which an as yet un-applied patch
> +set will need to wait for the NEXT+1 merge window. Of course there is no
> +obligation to ever except any given patchset, but if the review has not

s/except/accept/

> +concluded by this point the expectation the contributor should wait and

expectation is (that)

> +resubmit for the following merge window. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Geert Uytterhoeven Nov. 15, 2018, 7:59 a.m. UTC | #4
Hi Julia,

On Thu, Nov 15, 2018 at 6:48 AM Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Wed, 14 Nov 2018, Dan Williams wrote:
> > As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> > Profile is proposed as a way to reduce friction between committers and
> > maintainers and perhaps encourage conversations amongst maintainers
> > about best practice policies.
> >
> > The profile contains short answers to some of the common policy
> > questions a contributor might have, or that a maintainer might consider
> > formalizing. The current list of maintenance policies is:
> >
> > Overview: General introduction to maintaining the subsystem
> > Core: List of source files considered core
> > Leaf: List of source files that consume core functionality
> > Patches or Pull requests: Simple statement of expected submission format
> > Last -rc for new feature submissions: Expected lead time for submissions
> > Last -rc to merge features: Deadline for merge decisions
> > Non-author Ack / Review Tags Required: Patch review economics
> > Test Suite: Pass this suite before requesting inclusion
> > Resubmit Cadence: When to ping the maintainer
> > Trusted Reviewers: Help for triaging patches
> > Time Zone / Office Hours: When might a maintainer be available
> > Checkpatch / Style Cleanups: Policy on pure cleanup patches
> > Off-list review: Request for review gates
> > TODO: Potential development tasks up for grabs, or active focus areas
>
> How about patch subject lines?  What is the formula that should be used to
> transform the name(s) of the affected file(s) into an appropriate suject
> line?

Automating that may be difficult.
I always use "git log --oneline", and try to derive something sane
from its output.

Gr{oetje,eeting}s,

                        Geert
Jani Nikula Nov. 15, 2018, 8:38 a.m. UTC | #5
Cc: linux-doc

On Wed, 14 Nov 2018, Dan Williams <dan.j.williams@intel.com> wrote:
> As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> Profile is proposed as a way to reduce friction between committers and
> maintainers and perhaps encourage conversations amongst maintainers
> about best practice policies.
>
> The profile contains short answers to some of the common policy
> questions a contributor might have, or that a maintainer might consider
> formalizing. The current list of maintenance policies is:
>
> Overview: General introduction to maintaining the subsystem
> Core: List of source files considered core
> Leaf: List of source files that consume core functionality
> Patches or Pull requests: Simple statement of expected submission format
> Last -rc for new feature submissions: Expected lead time for submissions
> Last -rc to merge features: Deadline for merge decisions
> Non-author Ack / Review Tags Required: Patch review economics
> Test Suite: Pass this suite before requesting inclusion
> Resubmit Cadence: When to ping the maintainer
> Trusted Reviewers: Help for triaging patches
> Time Zone / Office Hours: When might a maintainer be available
> Checkpatch / Style Cleanups: Policy on pure cleanup patches
> Off-list review: Request for review gates
> TODO: Potential development tasks up for grabs, or active focus areas
>
> The goal of the Subsystem Profile is to set expectations for
> contributors and interim or replacement maintainers for a subsystem.

First of all, I welcome documentation efforts like this.

The cover letter mainly focuses on the maintainer aspect, and the
documentation is added to the maintainer handbook. However, here you set
the goal as setting expectations for contributors. The example nvdimm
profile in patch 3/3 addresses the reader as a new maintainer, yet goes
on to set expectations also for contributors, not just the maintainer.

I do think the documentation for contributors and maintainers/committers
should be kept separate. Most contributors will never care about the
documentation for the latter. We have Documentation/process for
contributors, and I think the audience of Documentation/maintainer
should be strictly maintainers.

In summary, I do think we need all of the documentation you propose, and
I appreciate you taking this on, but I think this should be split by
audience.

BR,
Jani.


>
> See Documentation/maintainer/subsystem-profile.rst for more details, and
> a follow-on example profile for the libnvdimm subsystem.
>
> [1]: https://linuxplumbersconf.org/event/2/contributions/59/
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Steve French <stfrench@microsoft.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tobin C. Harding <me@tobin.cc>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/maintainer/index.rst             |    1 
>  Documentation/maintainer/subsystem-profile.rst |  145 ++++++++++++++++++++++++
>  MAINTAINERS                                    |    4 +
>  3 files changed, 150 insertions(+)
>  create mode 100644 Documentation/maintainer/subsystem-profile.rst
>
> diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst
> index 2a14916930cb..1e6b1aaa6024 100644
> --- a/Documentation/maintainer/index.rst
> +++ b/Documentation/maintainer/index.rst
> @@ -11,4 +11,5 @@ additions to this manual.
>  
>     configure-git
>     pull-requests
> +   subsystem-profile
>  
> diff --git a/Documentation/maintainer/subsystem-profile.rst b/Documentation/maintainer/subsystem-profile.rst
> new file mode 100644
> index 000000000000..a74b624e0972
> --- /dev/null
> +++ b/Documentation/maintainer/subsystem-profile.rst
> @@ -0,0 +1,145 @@
> +.. _subsystemprofile:
> +
> +Subsystem Profile
> +=================
> +
> +The Subsystem Profile is a collection of policy positions that a
> +maintainer or maintainer team establishes for the their subsystem. While
> +there is a wide range of technical nuance on maintaining disparate
> +sections of the kernel, the Subsystem Profile documents a known set of
> +major process policies that vary between subsystems.  What follows is a
> +list of policy questions a maintainer can answer and include a document
> +in the kernel, or on an external website. It advertises to other
> +maintainers and contributors the local policy of the subsystem. Some
> +sections are optional like "Overview", "Off-list review", and "TODO".
> +The others are recommended for all subsystems to address, but no section
> +is mandatory. In addition there are no wrong answers, just document how
> +a subsystem typically operates. Note that the profile follows the
> +subsystem not the maintainer, i.e. there is no expectation that a
> +maintainer of multiple subsystems deploys the same policy across those
> +subsystems.
> +
> +
> +Overview
> +--------
> +In this optional section of the profile provide a free form overview of
> +the subsystem written as a hand-off document. In other words write a
> +note to someone that would receive the “keys to the castle” in the event
> +of extended or unexpected absence. “So, you have recently become the
> +maintainer of the XYZ subsystem, condolences, it is a thankless job,
> +here is the lay of the land.” Details to consider are the extended
> +details that are not included in MAINTAINERS, and not addressed by the
> +other profile questions below. For example details like, who has access
> +to the git tree, branches that are pulled into -next, relevant
> +specifications, issue trackers, and sensitive code areas. If available
> +the Overview should link to other subsystem documentation that may
> +clarify, re-iterate, emphasize / de-emphasize portions of the global
> +process documentation for contributors (CodingStyle, SubmittingPatches,
> +etc...).
> +
> +
> +Core
> +----
> +A list of F: tags (as described by MAINTAINERS) listing what the
> +maintainer considers to be core files. The review and lead time
> +constraints for 'core' code may be stricter given the increased
> +sensitivity and risk of change.
> +
> +
> +Patches or Pull requests
> +------------------------
> +Some subsystems allow contributors to send pull requests, most require
> +mailed patches. State “Patches only”, or “Pull requests accepted”.
> +
> +
> +Last -rc for new feature submissions
> +------------------------------------
> +New feature submissions targeting the next merge window should have
> +their first posting for consideration before this point. Patches that
> +are submitted after this point should be clear that they are targeting
> +the NEXT+1 merge window, or should come with sufficient justification
> +why they should be considered on an expedited schedule. A general
> +guideline is to set expectation with contributors that new feature
> +submissions should appear before -rc5. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.
> +
> +
> +Last -rc to merge features
> +--------------------------
> +Indicate to contributors the point at which an as yet un-applied patch
> +set will need to wait for the NEXT+1 merge window. Of course there is no
> +obligation to ever except any given patchset, but if the review has not
> +concluded by this point the expectation the contributor should wait and
> +resubmit for the following merge window. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.
> +
> +
> +Non-author Ack / Review Tags Required
> +-------------------------------------
> +Let contributors and other maintainers know whether they can expect to
> +see the maintainer self-commit patches without 3rd-party review. Some
> +subsystem developer communities are so small as to make this requirement
> +impractical. Others may have been bootstrapped by a submission of
> +self-reviewed code at the outset, but have since moved to a
> +non-author review-required stance. This section sets expectations on the
> +code-review economics in the subsystem. For example, can a contributor
> +trade review of a maintainer's, or other contributor's patches in
> +exchange for consideration of their own.
> +
> +
> +Test Suite
> +----------
> +Indicate the test suite all patches are expected to pass before being
> +submitted for inclusion consideration.
> +
> +
> +Resubmit Cadence
> +----------------
> +Define a rate at which a contributor should wait to resubmit a patchset
> +that has not yet received comments. A general guideline is to try to
> +meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
> +a patch set.
> +
> +
> +Trusted Reviewers
> +-----------------
> +While a maintainer / maintainer-team is expected to be reviewer of last
> +resort the review load is less onerous when distributed amongst
> +contributors and or a trusted set of individuals.  This section is
> +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> +reviewers that should always be copied on a patch submission, the
> +trusted reviewers here are individuals contributors can reach out to if
> +a few 'Resubmit Cadence' intervals have gone by without maintainer
> +action, or to otherwise consult for advice.
> +
> +
> +Time Zone / Office Hours
> +------------------------
> +Let contributors know the time of day when one or more maintainers are
> +usually actively monitoring the mailing list.
> +
> +
> +Checkpatch / Style Cleanups
> +---------------------------
> +For subsystems with long standing code bases it is reasonable to decline
> +to accept pure coding-style fixup patches. This is where you can let
> +contributors know “Standalone style-cleanups are welcome”,
> +“Style-cleanups to existing code only welcome with other feature
> +changes”, or “Standalone style-cleanups to existing code are not
> +welcome”.
> +
> +
> +Off-list review
> +---------------
> +A maintainer may optionally require that contributors seek prior review
> +of patches before initial submission for upstream. For example,
> +“Developers from organization X, please seek internal review before
> +requesting upstream review”. This policy identifies occasions where a
> +maintainer wants to reflect some of the review load back to an
> +organization.
> +
> +
> +TODO
> +----
> +In this optional section include a list of work items that might be
> +suitable for onboarding a new developer to the subsystem.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83b7b3943a12..bb4a83a7684d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -99,6 +99,10 @@ Descriptions of section entries:
>  	   Obsolete:	Old code. Something tagged obsolete generally means
>  			it has been replaced by a better system and you
>  			should be using that.
> +	P: Subsystem Profile document for the maintainer entry.  This
> +	   is either an in-tree file or a URI to a document.  The
> +	   contents of a Subsystem Profile are described in
> +	   Documentation/maintainer/subsystem-profile.rst.
>  	F: Files and directories with wildcard patterns.
>  	   A trailing slash includes all files and subdirectory files.
>  	   F:	drivers/net/	all files in and below drivers/net
>
> _______________________________________________
> Ksummit-discuss mailing list
> Ksummit-discuss@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
Julia Lawall Nov. 15, 2018, 1:47 p.m. UTC | #6
On Thu, 15 Nov 2018, Geert Uytterhoeven wrote:

> Hi Julia,
>
> On Thu, Nov 15, 2018 at 6:48 AM Julia Lawall <julia.lawall@lip6.fr> wrote:
> > On Wed, 14 Nov 2018, Dan Williams wrote:
> > > As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> > > Profile is proposed as a way to reduce friction between committers and
> > > maintainers and perhaps encourage conversations amongst maintainers
> > > about best practice policies.
> > >
> > > The profile contains short answers to some of the common policy
> > > questions a contributor might have, or that a maintainer might consider
> > > formalizing. The current list of maintenance policies is:
> > >
> > > Overview: General introduction to maintaining the subsystem
> > > Core: List of source files considered core
> > > Leaf: List of source files that consume core functionality
> > > Patches or Pull requests: Simple statement of expected submission format
> > > Last -rc for new feature submissions: Expected lead time for submissions
> > > Last -rc to merge features: Deadline for merge decisions
> > > Non-author Ack / Review Tags Required: Patch review economics
> > > Test Suite: Pass this suite before requesting inclusion
> > > Resubmit Cadence: When to ping the maintainer
> > > Trusted Reviewers: Help for triaging patches
> > > Time Zone / Office Hours: When might a maintainer be available
> > > Checkpatch / Style Cleanups: Policy on pure cleanup patches
> > > Off-list review: Request for review gates
> > > TODO: Potential development tasks up for grabs, or active focus areas
> >
> > How about patch subject lines?  What is the formula that should be used to
> > transform the name(s) of the affected file(s) into an appropriate suject
> > line?
>
> Automating that may be difficult.
> I always use "git log --oneline", and try to derive something sane
> from its output.

Yes, I do likewise.  But there may be some subsystems for which it would
be possible to come up with a more specific policy.  The advantage of what
is proposed here is that it is not necessary to come up with a single
formula that works everywhere.  Even a description in English could be
helpful.

Other subsystem specific properties such as what tree to work on, whether
to CC stable, ordering of variables or header files, etc could also be
useful to mention.  Anything that will cause a maintainer to immediately
reject a patch for syntactic reasons.

julia
Mauro Carvalho Chehab Nov. 15, 2018, 3:44 p.m. UTC | #7
Em Wed, 14 Nov 2018 20:53:25 -0800
Dan Williams <dan.j.williams@intel.com> escreveu:

> As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> Profile is proposed as a way to reduce friction between committers and
> maintainers and perhaps encourage conversations amongst maintainers
> about best practice policies.
> 
> The profile contains short answers to some of the common policy
> questions a contributor might have, or that a maintainer might consider
> formalizing. The current list of maintenance policies is:
> 
> Overview: General introduction to maintaining the subsystem
> Core: List of source files considered core
> Leaf: List of source files that consume core functionality
> Patches or Pull requests: Simple statement of expected submission format
> Last -rc for new feature submissions: Expected lead time for submissions
> Last -rc to merge features: Deadline for merge decisions
> Non-author Ack / Review Tags Required: Patch review economics
> Test Suite: Pass this suite before requesting inclusion
> Resubmit Cadence: When to ping the maintainer
> Trusted Reviewers: Help for triaging patches
> Time Zone / Office Hours: When might a maintainer be available
> Checkpatch / Style Cleanups: Policy on pure cleanup patches
> Off-list review: Request for review gates
> TODO: Potential development tasks up for grabs, or active focus areas
> 
> The goal of the Subsystem Profile is to set expectations for
> contributors and interim or replacement maintainers for a subsystem.
> 
> See Documentation/maintainer/subsystem-profile.rst for more details, and
> a follow-on example profile for the libnvdimm subsystem.
> 
> [1]: https://linuxplumbersconf.org/event/2/contributions/59/
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Steve French <stfrench@microsoft.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tobin C. Harding <me@tobin.cc>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Ok, I sort of followed the proposed model, adding a documentation for
the media subsystem using the template.

I'm pretty sure this is incomplete and more work would be needed.
So, for now, this is just an example.

Also, I didn't test building it on Sphinx (nor added any reference for it
at MAINTAINERS), as my main goal here was to see how the model would fit
for us.

I noticed a few issues there:

1) Describing the "core" files. The media subsystem is complex: depending on
the device type, we have different APIs and different cores. So, our core
is actually a set of different cores. I ended by adding sub-sections.

Also, several things were not described there. For example, we store
DVB frontend drivers on an specific directory. We have drivers there organized
by bus type. So, one directory for I2C, one for USB, one for PCI, ...

It should probably make sense to be able to tell about that.

So, I would actually prefer to have, at the profile, a "files" section,
instead of "core".

2) As I said before, on media, we have sub-maintainers. Not sure how to
describe them on this template.

3) I noticed one big missing thing there: in the case of media, we are
responsible also to maintaining media staging drivers, that goes at
drivers/staging/media. IMO, it would be important to be able to describe
who maintains staging stuff - e. g. if the subsystem maintainers prefer
to let staging up to staging maintainers or if they'll maintain themselves.

In the latter, it probably makes sense to describe any specific thing
related to it (where staging will be at the tree, are there any special
rules for them?)

Anyway, RFC patch follows.

-

[PATCH] [RFC] Add a system profile description for media subsystem

This RFC aligns with current Dan's proposal for having subsystem
specific ruleset stored at the Kernel tree.

On this initial RFC, I opted to not add the reviewers e-mail
(adding just a "<>") as a boilerplate. If we decide keeping emails
there, I'll add them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/Documentation/media/subsystem-profile.rst b/Documentation/media/subsystem-profile.rst
new file mode 100644
index 000000000000..7a5d6f691d05
--- /dev/null
+++ b/Documentation/media/subsystem-profile.rst
@@ -0,0 +1,186 @@
+Media Subsystem Profile
+=======================
+
+Overview
+--------
+
+The media subsystem cover support for a variety of devices: stream
+capture, analog and digital TV, cameras, remote controllers, HDMI CEC
+and media pipeline control.
+
+Both our userspace and Kernel APIs are documented and should be kept in
+sync with the API changes. It means that all patches that add new
+features to the subsystem should also bring changes to the corresponding
+API files.
+
+Also, patches for device drivers that changes the Open Firmware/Device
+Tree bindings should be reviewed by the Device Tree maintainers.
+
+Due to the size and wide scope of the media subsystem, our
+maintainership model is to have sub-maintainers that have a broad
+knowledge of an specific aspect of the subsystem. It is a
+sub-maintainers task to review the patches, providing feedback to users
+if the patches are following the subsystem rules and are properly using
+the media internal and external APIs.
+
+We have a set of compliance tools at https://git.linuxtv.org/v4l-utils.git/
+that should be used in order to check if the drivers are properly
+implementing the media APIs.
+
+Patches for the media subsystem should be sent to the media mailing list
+at linux-media@vger.kernel.org as plain text only e-mail. emails with
+HTML will be automacially rejected by the mail server.
+
+Our workflow is heavily based on Patchwork, meaning that, once a patch
+is submitted, it should appear at:
+
+   - https://patchwork.linuxtv.org/project/linux-media/list/
+
+If it doesn't automatically appear there after a few minutes, then
+probably something got wrong on your submission. Please check if the
+email is in plain text only and if your emailer is not mangling with
+whitespaces before complaining or submit it again.
+
+Core
+----
+
+Documentation
++++++++++++++
+
+F: Documentation/media
+
+Kernelspace API headers
++++++++++++++++++++++++
+
+F: include/media/*.h
+
+Digital TV Core
++++++++++++++++
+
+F: drivers/media/dvb-core
+
+HDMI CEC Core
++++++++++++++
+
+F: drivers/media/cec
+
+Media Controller Core
++++++++++++++++++++++
+
+F: drivers/media/media-\*.[ch]
+
+Remote Controller Core
+++++++++++++++++++++++
+
+F: drivers/media/rc/rc-core-priv.h
+F: drivers/media/rc/rc-ir-raw.c
+F: drivers/media/rc/rc-main.c
+F: drivers/media/rc/ir\*-decoder.c
+F: drivers/media/rc/lirc_dev.c
+
+Video4linux Core
+++++++++++++++++
+
+F: drivers/media/v4l2-core
+
+Patches or Pull requests
+------------------------
+
+All patches should be submitted via e-mail for review. We use
+pull requests on our workflow between sub-maintainers and the
+maintainer.
+
+
+Last day for new feature submissions
+------------------------------------
+
+Before -rc5
+
+
+Last day to merge features
+--------------------------
+
+Before -rc7
+
+
+Non-author Ack / Review Tags Required
+-------------------------------------
+
+Not required, but desirable
+
+
+Test Suite
+----------
+
+Use the several *-compliance tools that are part of the v4l-utils
+package.
+
+
+Trusted Reviewers
+-----------------
+
+Sub-maintainers
++++++++++++++++
+
+At the media subsystem, we have a group of senior developers that are
+responsible for doing the code reviews at the drivers (called
+sub-maintainers), and another senior developer responsible for the
+subsystem as a hole. For core changes, whenever possible, multiple
+media (sub-)maintainers do the review.
+
+The sub-maintainers work on specific areas of the subsystem, as
+described below:
+
+- Sensor drivers
+
+  R: Sakari Ailus <>
+
+- V4L2 drivers
+
+  R: Hans Verkuil <>
+
+- Media controller drivers
+
+  R: Laurent Pinchart <>
+
+- HDMI CEC
+
+- Remote Controllers
+
+  R: Sean Young <>
+
+- Digital TV
+
+  R: Michael Krufky <>
+  R: Sean Young <>
+
+
+Resubmit Cadence
+----------------
+
+Provided that your patch is at https://patchwork.linuxtv.org, it should
+be sooner or later handled, so you don't need to re-submit a patch.
+
+Please notice that the media subsystem is a high traffic one, so it
+could take a while for us to be able to review your patches. Feel free
+to ping if you don't get a feedback on a couple of weeks.
+
+Time Zone / Office Hours
+------------------------
+
+Media developers are distributed all around the globe. So, don't assume
+that we're on your time zone. We usually don't work on local holidays or
+at weekends. Please also notice that, during the Kernel merge window,
+we're usually busy ensuring that everything goes smoothly, meaning that
+we usually have a lot of patches waiting for review just after that. So
+you should expect a higher delay during the merge window and one week
+before/after it.
+
+
+Checkpatch / Style cleanups
+---------------------------
+
+Standalone style-cleanups are welcome, but they should be grouped per
+directory. So, for example, if you're doing a cleanup at drivers
+under drivers/media, please send a single patch for all drivers under
+drivers/media/pci, another one for drivers/media/usb and so on.





Cheers,
Mauro
Bird, Tim Nov. 15, 2018, 6:03 p.m. UTC | #8
> -----Original Message-----
> From: Jani Nikula on Thursday, November 15, 2018 12:39 AM 
> 
> Cc: linux-doc
> 
> On Wed, 14 Nov 2018, Dan Williams <dan.j.williams@intel.com> wrote:
> > As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> > Profile is proposed as a way to reduce friction between committers and
> > maintainers and perhaps encourage conversations amongst maintainers
> > about best practice policies.
> >
> > The profile contains short answers to some of the common policy
> > questions a contributor might have, or that a maintainer might consider
> > formalizing. The current list of maintenance policies is:
> >
> > Overview: General introduction to maintaining the subsystem
> > Core: List of source files considered core
> > Leaf: List of source files that consume core functionality
> > Patches or Pull requests: Simple statement of expected submission format
> > Last -rc for new feature submissions: Expected lead time for submissions
> > Last -rc to merge features: Deadline for merge decisions
> > Non-author Ack / Review Tags Required: Patch review economics
> > Test Suite: Pass this suite before requesting inclusion
> > Resubmit Cadence: When to ping the maintainer
> > Trusted Reviewers: Help for triaging patches
> > Time Zone / Office Hours: When might a maintainer be available
> > Checkpatch / Style Cleanups: Policy on pure cleanup patches
> > Off-list review: Request for review gates
> > TODO: Potential development tasks up for grabs, or active focus areas
> >
> > The goal of the Subsystem Profile is to set expectations for
> > contributors and interim or replacement maintainers for a subsystem.
> 
> First of all, I welcome documentation efforts like this.
> 
> The cover letter mainly focuses on the maintainer aspect, and the
> documentation is added to the maintainer handbook. However, here you set
> the goal as setting expectations for contributors. The example nvdimm
> profile in patch 3/3 addresses the reader as a new maintainer, yet goes
> on to set expectations also for contributors, not just the maintainer.
> 
> I do think the documentation for contributors and maintainers/committers
> should be kept separate. Most contributors will never care about the
> documentation for the latter. We have Documentation/process for
> contributors, and I think the audience of Documentation/maintainer
> should be strictly maintainers.
> 
> In summary, I do think we need all of the documentation you propose, and
> I appreciate you taking this on, but I think this should be split by
> audience.

I think there is a spectrum of audiences for this document, including contributors,
reviewers, and maintainers.  Some of this information documents  the "social API" between
these groups.  So, I think it's handy to have it in one place, viewable by all parties.
However, designating items that are of special interest to maintainers, or reviewers
or contributors is worthwhile.  Maybe having sections in a single document would
serve that purpose?

Put another way, there are pieces of information that are an agreement between
maintainers and contributors, that both should probably be reminded of.  Maintaining
such information in 2 places would be a pain, and prone to getting out-of-sync.

Also,
I think over time we want to contributors to feel comfortable becoming reviewers
and maintainers, so it's good for them to learn about the processes that the
maintainers are using.  I fear if we kept the information
in role-specific documents, it wouldn't promote this progression.
 -- Tim
Tobin C. Harding Nov. 15, 2018, 11:56 p.m. UTC | #9
On Thu, Nov 15, 2018 at 10:38:44AM +0200, Jani Nikula wrote:
> 
> Cc: linux-doc
> 
> On Wed, 14 Nov 2018, Dan Williams <dan.j.williams@intel.com> wrote:
> > As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> > Profile is proposed as a way to reduce friction between committers and
> > maintainers and perhaps encourage conversations amongst maintainers
> > about best practice policies.
> >
> > The profile contains short answers to some of the common policy
> > questions a contributor might have, or that a maintainer might consider
> > formalizing. The current list of maintenance policies is:
> >
> > Overview: General introduction to maintaining the subsystem
> > Core: List of source files considered core
> > Leaf: List of source files that consume core functionality
> > Patches or Pull requests: Simple statement of expected submission format
> > Last -rc for new feature submissions: Expected lead time for submissions
> > Last -rc to merge features: Deadline for merge decisions
> > Non-author Ack / Review Tags Required: Patch review economics
> > Test Suite: Pass this suite before requesting inclusion
> > Resubmit Cadence: When to ping the maintainer
> > Trusted Reviewers: Help for triaging patches
> > Time Zone / Office Hours: When might a maintainer be available
> > Checkpatch / Style Cleanups: Policy on pure cleanup patches
> > Off-list review: Request for review gates
> > TODO: Potential development tasks up for grabs, or active focus areas
> >
> > The goal of the Subsystem Profile is to set expectations for
> > contributors and interim or replacement maintainers for a subsystem.
> 
> First of all, I welcome documentation efforts like this.
> 
> The cover letter mainly focuses on the maintainer aspect, and the
> documentation is added to the maintainer handbook. However, here you set
> the goal as setting expectations for contributors. The example nvdimm
> profile in patch 3/3 addresses the reader as a new maintainer, yet goes
> on to set expectations also for contributors, not just the maintainer.
> 
> I do think the documentation for contributors and maintainers/committers
> should be kept separate. Most contributors will never care about the
> documentation for the latter. We have Documentation/process for
> contributors, and I think the audience of Documentation/maintainer
> should be strictly maintainers.
> 
> In summary, I do think we need all of the documentation you propose, and
> I appreciate you taking this on, but I think this should be split by
> audience.

I got confused by this at first also Jani.  This document is a template
for use by maintainers.  The files created from the template (by a
subsystem maintainer) are for contributors.  So I believe this document
is in the correct place.

Hope this helps to clarify.

     Tobin
Frank Rowand Nov. 16, 2018, 12:11 a.m. UTC | #10
On 11/14/18 8:53 PM, Dan Williams wrote:
> As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> Profile is proposed as a way to reduce friction between committers and
> maintainers and perhaps encourage conversations amongst maintainers
> about best practice policies.

Thanks for working on this.


> The profile contains short answers to some of the common policy
> questions a contributor might have, or that a maintainer might consider
> formalizing. The current list of maintenance policies is:
> 
> Overview: General introduction to maintaining the subsystem
> Core: List of source files considered core
> Leaf: List of source files that consume core functionality
> Patches or Pull requests: Simple statement of expected submission format
> Last -rc for new feature submissions: Expected lead time for submissions
> Last -rc to merge features: Deadline for merge decisions
> Non-author Ack / Review Tags Required: Patch review economics
> Test Suite: Pass this suite before requesting inclusion
> Resubmit Cadence: When to ping the maintainer
> Trusted Reviewers: Help for triaging patches
> Time Zone / Office Hours: When might a maintainer be available
> Checkpatch / Style Cleanups: Policy on pure cleanup patches
> Off-list review: Request for review gates
> TODO: Potential development tasks up for grabs, or active focus areas
> 
> The goal of the Subsystem Profile is to set expectations for
> contributors and interim or replacement maintainers for a subsystem.
> 
> See Documentation/maintainer/subsystem-profile.rst for more details, and
> a follow-on example profile for the libnvdimm subsystem.
> 
> [1]: https://linuxplumbersconf.org/event/2/contributions/59/
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Steve French <stfrench@microsoft.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Tobin C. Harding <me@tobin.cc>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/maintainer/index.rst             |    1 
>  Documentation/maintainer/subsystem-profile.rst |  145 ++++++++++++++++++++++++
>  MAINTAINERS                                    |    4 +
>  3 files changed, 150 insertions(+)
>  create mode 100644 Documentation/maintainer/subsystem-profile.rst
> 
> diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst
> index 2a14916930cb..1e6b1aaa6024 100644
> --- a/Documentation/maintainer/index.rst
> +++ b/Documentation/maintainer/index.rst
> @@ -11,4 +11,5 @@ additions to this manual.
>  
>     configure-git
>     pull-requests
> +   subsystem-profile
>  
> diff --git a/Documentation/maintainer/subsystem-profile.rst b/Documentation/maintainer/subsystem-profile.rst
> new file mode 100644
> index 000000000000..a74b624e0972
> --- /dev/null
> +++ b/Documentation/maintainer/subsystem-profile.rst
> @@ -0,0 +1,145 @@
> +.. _subsystemprofile:
> +
> +Subsystem Profile
> +=================
> +
> +The Subsystem Profile is a collection of policy positions that a
> +maintainer or maintainer team establishes for the their subsystem. While
> +there is a wide range of technical nuance on maintaining disparate
> +sections of the kernel, the Subsystem Profile documents a known set of
> +major process policies that vary between subsystems.  What follows is a
> +list of policy questions a maintainer can answer and include a document
> +in the kernel, or on an external website. It advertises to other
> +maintainers and contributors the local policy of the subsystem. Some
> +sections are optional like "Overview", "Off-list review", and "TODO".
> +The others are recommended for all subsystems to address, but no section
> +is mandatory. In addition there are no wrong answers, just document how
> +a subsystem typically operates. Note that the profile follows the
> +subsystem not the maintainer, i.e. there is no expectation that a
> +maintainer of multiple subsystems deploys the same policy across those
> +subsystems.
> +
> +
> +Overview
> +--------
> +In this optional section of the profile provide a free form overview of
> +the subsystem written as a hand-off document. In other words write a
> +note to someone that would receive the “keys to the castle” in the event
> +of extended or unexpected absence. “So, you have recently become the
> +maintainer of the XYZ subsystem, condolences, it is a thankless job,
> +here is the lay of the land.” Details to consider are the extended
> +details that are not included in MAINTAINERS, and not addressed by the
> +other profile questions below. For example details like, who has access
> +to the git tree, branches that are pulled into -next, relevant
> +specifications, issue trackers, and sensitive code areas. If available
> +the Overview should link to other subsystem documentation that may
> +clarify, re-iterate, emphasize / de-emphasize portions of the global
> +process documentation for contributors (CodingStyle, SubmittingPatches,
> +etc...).
> +
> +
> +Core
> +----
> +A list of F: tags (as described by MAINTAINERS) listing what the
> +maintainer considers to be core files. The review and lead time
> +constraints for 'core' code may be stricter given the increased
> +sensitivity and risk of change.
> +
> +
> +Patches or Pull requests
> +------------------------
> +Some subsystems allow contributors to send pull requests, most require
> +mailed patches. State “Patches only”, or “Pull requests accepted”.

This may create some confusion if only "Pull requests accepted" is the
contents of this section.  My understanding has been that all changes
should be available on a mail list for review before being accepted
by the maintainer, even if eventually the final version of the series
that is accepted is applied by the maintainer as a pull instead of
via applying patches.

Is my "must appear on a mail list" understanding correct?


> +
> +
> +Last -rc for new feature submissions
> +------------------------------------
> +New feature submissions targeting the next merge window should have
> +their first posting for consideration before this point. Patches that
> +are submitted after this point should be clear that they are targeting
> +the NEXT+1 merge window, or should come with sufficient justification
> +why they should be considered on an expedited schedule. A general
> +guideline is to set expectation with contributors that new feature
> +submissions should appear before -rc5. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.

I would expect the -rc to vary based on the patch series size, complexity,
risk, number of revisions that will occur, how controversial, number of
-rc releases before Linus chooses to release, etc.  You would need a
crystal ball to predict much of this.  I could see being able to provide
a good estimate of this value for a relatively simple patch.


> +
> +
> +Last -rc to merge features
> +--------------------------
> +Indicate to contributors the point at which an as yet un-applied patch
> +set will need to wait for the NEXT+1 merge window. Of course there is no
> +obligation to ever except any given patchset, but if the review has not
> +concluded by this point the expectation the contributor should wait and
> +resubmit for the following merge window. The answer may be different for
> +'Core:' files, include a second entry prefixed with 'Core:' if so.

Same comment as for the previous section, with the additional complexity
that each unique patch series should bake in -next.


> +
> +
> +Non-author Ack / Review Tags Required
> +-------------------------------------

The title of this section seems a bit different than the description
below.  A more aligned title would be something like "Maintainer
self-commit review requirements".


> +Let contributors and other maintainers know whether they can expect to
> +see the maintainer self-commit patches without 3rd-party review. Some
> +subsystem developer communities are so small as to make this requirement
> +impractical. Others may have been bootstrapped by a submission of
> +self-reviewed code at the outset, but have since moved to a
> +non-author review-required stance. This section sets expectations on the
> +code-review economics in the subsystem. For example, can a contributor
> +trade review of a maintainer's, or other contributor's patches in
> +exchange for consideration of their own.

Then another section (which is the one I expected when I slightly
mis-read the section title) would be: Review Tags Requirements",
which would discuss what type of review is expected, encouraged,
or required.


> +
> +
> +Test Suite
> +----------
> +Indicate the test suite all patches are expected to pass before being
> +submitted for inclusion consideration.
> +
> +
> +Resubmit Cadence
> +----------------
> +Define a rate at which a contributor should wait to resubmit a patchset
> +that has not yet received comments. A general guideline is to try to
> +meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
> +a patch set.

Or ping instead of resubmitting?  If you resubmit the same series with
an unchanged version then comments could be split across multiple
email threads.  If you resubmit the same series with a new version
number, the same comment split can occur plus it can be confusing
that two versions have the same contents.  I suspect that there may be
some maintainers who do prefer a resubmit, but that is just wild
conjecture on my part.


> +
> +
> +Trusted Reviewers
> +-----------------
> +While a maintainer / maintainer-team is expected to be reviewer of last
> +resort the review load is less onerous when distributed amongst
> +contributors and or a trusted set of individuals.  This section is
> +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> +reviewers that should always be copied on a patch submission, the
> +trusted reviewers here are individuals contributors can reach out to if
> +a few 'Resubmit Cadence' intervals have gone by without maintainer
> +action, or to otherwise consult for advice.

This seems redundant with the MAINTAINERS reviewers list.  It seems like
the role specified in this section is more of an ombudsman or developer
advocate who can assist with the review and/or accept flow if the
maintainer is being slow to respond.


> +
> +
> +Time Zone / Office Hours
> +------------------------
> +Let contributors know the time of day when one or more maintainers are
> +usually actively monitoring the mailing list.

I would strike "actively monitoring the mailing list".  To me, it should
be what are the hours of the day that the maintainer might happen to poll
(or might receive an interrupt) from the appropriate communications
channels (could be IRC, could be email, etc).

For my area, I would want to say something like: I tend to be active
between 17:00 UTC (18:00 UTC when daylight savings) and 25:00 (26:00),
but often will check for urgent or brief items up until 07:00 (08:00).
I interact with email via a poll model.  I interact with IRC via a
pull model and often overlook IRC activity for multiple days).

-Frank


> +
> +
> +Checkpatch / Style Cleanups
> +---------------------------
> +For subsystems with long standing code bases it is reasonable to decline
> +to accept pure coding-style fixup patches. This is where you can let
> +contributors know “Standalone style-cleanups are welcome”,
> +“Style-cleanups to existing code only welcome with other feature
> +changes”, or “Standalone style-cleanups to existing code are not
> +welcome”.
> +
> +
> +Off-list review
> +---------------
> +A maintainer may optionally require that contributors seek prior review
> +of patches before initial submission for upstream. For example,
> +“Developers from organization X, please seek internal review before
> +requesting upstream review”. This policy identifies occasions where a
> +maintainer wants to reflect some of the review load back to an
> +organization.
> +
> +
> +TODO
> +----
> +In this optional section include a list of work items that might be
> +suitable for onboarding a new developer to the subsystem.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83b7b3943a12..bb4a83a7684d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -99,6 +99,10 @@ Descriptions of section entries:
>  	   Obsolete:	Old code. Something tagged obsolete generally means
>  			it has been replaced by a better system and you
>  			should be using that.
> +	P: Subsystem Profile document for the maintainer entry.  This
> +	   is either an in-tree file or a URI to a document.  The
> +	   contents of a Subsystem Profile are described in
> +	   Documentation/maintainer/subsystem-profile.rst.
>  	F: Files and directories with wildcard patterns.
>  	   A trailing slash includes all files and subdirectory files.
>  	   F:	drivers/net/	all files in and below drivers/net
> 
> _______________________________________________
> Ksummit-discuss mailing list
> Ksummit-discuss@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
>
Mauro Carvalho Chehab Nov. 16, 2018, 12:04 p.m. UTC | #11
Em Thu, 15 Nov 2018 16:11:59 -0800
Frank Rowand <frowand.list@gmail.com> escreveu:

> On 11/14/18 8:53 PM, Dan Williams wrote:
> > As presented at the 2018 Linux Plumbers conference [1], the Subsystem
> > Profile is proposed as a way to reduce friction between committers and
> > maintainers and perhaps encourage conversations amongst maintainers
> > about best practice policies.  
> 
> Thanks for working on this.
> 
> 
> > The profile contains short answers to some of the common policy
> > questions a contributor might have, or that a maintainer might consider
> > formalizing. The current list of maintenance policies is:
> > 
> > Overview: General introduction to maintaining the subsystem
> > Core: List of source files considered core
> > Leaf: List of source files that consume core functionality
> > Patches or Pull requests: Simple statement of expected submission format
> > Last -rc for new feature submissions: Expected lead time for submissions
> > Last -rc to merge features: Deadline for merge decisions
> > Non-author Ack / Review Tags Required: Patch review economics
> > Test Suite: Pass this suite before requesting inclusion
> > Resubmit Cadence: When to ping the maintainer
> > Trusted Reviewers: Help for triaging patches
> > Time Zone / Office Hours: When might a maintainer be available
> > Checkpatch / Style Cleanups: Policy on pure cleanup patches
> > Off-list review: Request for review gates
> > TODO: Potential development tasks up for grabs, or active focus areas
> > 
> > The goal of the Subsystem Profile is to set expectations for
> > contributors and interim or replacement maintainers for a subsystem.
> > 
> > See Documentation/maintainer/subsystem-profile.rst for more details, and
> > a follow-on example profile for the libnvdimm subsystem.
> > 
> > [1]: https://linuxplumbersconf.org/event/2/contributions/59/
> > 
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: Steve French <stfrench@microsoft.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Tobin C. Harding <me@tobin.cc>
> > Cc: Olof Johansson <olof@lixom.net>
> > Cc: Martin K. Petersen <martin.petersen@oracle.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Joe Perches <joe@perches.com>
> > Cc: Dmitry Vyukov <dvyukov@google.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  Documentation/maintainer/index.rst             |    1 
> >  Documentation/maintainer/subsystem-profile.rst |  145 ++++++++++++++++++++++++
> >  MAINTAINERS                                    |    4 +
> >  3 files changed, 150 insertions(+)
> >  create mode 100644 Documentation/maintainer/subsystem-profile.rst
> > 
> > diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst
> > index 2a14916930cb..1e6b1aaa6024 100644
> > --- a/Documentation/maintainer/index.rst
> > +++ b/Documentation/maintainer/index.rst
> > @@ -11,4 +11,5 @@ additions to this manual.
> >  
> >     configure-git
> >     pull-requests
> > +   subsystem-profile
> >  
> > diff --git a/Documentation/maintainer/subsystem-profile.rst b/Documentation/maintainer/subsystem-profile.rst
> > new file mode 100644
> > index 000000000000..a74b624e0972
> > --- /dev/null
> > +++ b/Documentation/maintainer/subsystem-profile.rst
> > @@ -0,0 +1,145 @@
> > +.. _subsystemprofile:
> > +
> > +Subsystem Profile
> > +=================
> > +
> > +The Subsystem Profile is a collection of policy positions that a
> > +maintainer or maintainer team establishes for the their subsystem. While
> > +there is a wide range of technical nuance on maintaining disparate
> > +sections of the kernel, the Subsystem Profile documents a known set of
> > +major process policies that vary between subsystems.  What follows is a
> > +list of policy questions a maintainer can answer and include a document
> > +in the kernel, or on an external website. It advertises to other
> > +maintainers and contributors the local policy of the subsystem. Some
> > +sections are optional like "Overview", "Off-list review", and "TODO".
> > +The others are recommended for all subsystems to address, but no section
> > +is mandatory. In addition there are no wrong answers, just document how
> > +a subsystem typically operates. Note that the profile follows the
> > +subsystem not the maintainer, i.e. there is no expectation that a
> > +maintainer of multiple subsystems deploys the same policy across those
> > +subsystems.
> > +
> > +
> > +Overview
> > +--------
> > +In this optional section of the profile provide a free form overview of
> > +the subsystem written as a hand-off document. In other words write a
> > +note to someone that would receive the “keys to the castle” in the event
> > +of extended or unexpected absence. “So, you have recently become the
> > +maintainer of the XYZ subsystem, condolences, it is a thankless job,
> > +here is the lay of the land.” Details to consider are the extended
> > +details that are not included in MAINTAINERS, and not addressed by the
> > +other profile questions below. For example details like, who has access
> > +to the git tree, branches that are pulled into -next, relevant
> > +specifications, issue trackers, and sensitive code areas. If available
> > +the Overview should link to other subsystem documentation that may
> > +clarify, re-iterate, emphasize / de-emphasize portions of the global
> > +process documentation for contributors (CodingStyle, SubmittingPatches,
> > +etc...).
> > +
> > +
> > +Core
> > +----
> > +A list of F: tags (as described by MAINTAINERS) listing what the
> > +maintainer considers to be core files. The review and lead time
> > +constraints for 'core' code may be stricter given the increased
> > +sensitivity and risk of change.
> > +
> > +
> > +Patches or Pull requests
> > +------------------------
> > +Some subsystems allow contributors to send pull requests, most require
> > +mailed patches. State “Patches only”, or “Pull requests accepted”.  
> 
> This may create some confusion if only "Pull requests accepted" is the
> contents of this section.  My understanding has been that all changes
> should be available on a mail list for review before being accepted
> by the maintainer, even if eventually the final version of the series
> that is accepted is applied by the maintainer as a pull instead of
> via applying patches.
> 
> Is my "must appear on a mail list" understanding correct?

I guess this is true on almost all subsystems that accept pull requests.

It could not be true if some subsystem moves to Github/Gitlab.

> > +
> > +
> > +Last -rc for new feature submissions
> > +------------------------------------
> > +New feature submissions targeting the next merge window should have
> > +their first posting for consideration before this point. Patches that
> > +are submitted after this point should be clear that they are targeting
> > +the NEXT+1 merge window, or should come with sufficient justification
> > +why they should be considered on an expedited schedule. A general
> > +guideline is to set expectation with contributors that new feature
> > +submissions should appear before -rc5. The answer may be different for
> > +'Core:' files, include a second entry prefixed with 'Core:' if so.  
> 
> I would expect the -rc to vary based on the patch series size, complexity,
> risk, number of revisions that will occur, how controversial, number of
> -rc releases before Linus chooses to release, etc.  You would need a
> crystal ball to predict much of this.  I could see being able to provide
> a good estimate of this value for a relatively simple patch.

That's only partially true. I mean, the usual flux is to go up to -rc7.
After that, the final version is usually merged (except when something
goes wrong).

Anyway, I guess nobody would complain if the maintainers do an exception
here and accept more patches when they know that the last rc version
won't be -rc7, but, instead, -rc8 or -rc9.

This is a general ruleset that describes the usual behavior, telling the
developers the expected behavior. If the maintainers can do more on some
particular development cycle, it should be fine.

> 
> > +
> > +
> > +Last -rc to merge features
> > +--------------------------
> > +Indicate to contributors the point at which an as yet un-applied patch
> > +set will need to wait for the NEXT+1 merge window. Of course there is no
> > +obligation to ever except any given patchset, but if the review has not
> > +concluded by this point the expectation the contributor should wait and
> > +resubmit for the following merge window. The answer may be different for
> > +'Core:' files, include a second entry prefixed with 'Core:' if so.  
> 
> Same comment as for the previous section, with the additional complexity
> that each unique patch series should bake in -next.
> 
> 
> > +
> > +
> > +Non-author Ack / Review Tags Required
> > +-------------------------------------  
> 
> The title of this section seems a bit different than the description
> below.  A more aligned title would be something like "Maintainer
> self-commit review requirements".
> 
> 
> > +Let contributors and other maintainers know whether they can expect to
> > +see the maintainer self-commit patches without 3rd-party review. Some
> > +subsystem developer communities are so small as to make this requirement
> > +impractical. Others may have been bootstrapped by a submission of
> > +self-reviewed code at the outset, but have since moved to a
> > +non-author review-required stance. This section sets expectations on the
> > +code-review economics in the subsystem. For example, can a contributor
> > +trade review of a maintainer's, or other contributor's patches in
> > +exchange for consideration of their own.  
> 
> Then another section (which is the one I expected when I slightly
> mis-read the section title) would be: Review Tags Requirements",
> which would discuss what type of review is expected, encouraged,
> or required.
> 
> 
> > +
> > +
> > +Test Suite
> > +----------
> > +Indicate the test suite all patches are expected to pass before being
> > +submitted for inclusion consideration.
> > +
> > +
> > +Resubmit Cadence
> > +----------------
> > +Define a rate at which a contributor should wait to resubmit a patchset
> > +that has not yet received comments. A general guideline is to try to
> > +meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
> > +a patch set.  
> 
> Or ping instead of resubmitting?  If you resubmit the same series with
> an unchanged version then comments could be split across multiple
> email threads.  If you resubmit the same series with a new version
> number, the same comment split can occur plus it can be confusing
> that two versions have the same contents.  I suspect that there may be
> some maintainers who do prefer a resubmit, but that is just wild
> conjecture on my part.

That depends on how maintainers handle patches. Those subsystems that
use patchwork (like media) don't really require anyone to resubmit
patches. They're all stored there at patchwork database.

My workflow is that, if I receive two patches from the same person with
the same subject, I simply mark the first one as obsoleted, as usually 
the second one is a new version, and reviewers should need some
time to handle it. 

In other words, re-sending a patch may actually delay its handling, as
the second version will be later on my input queue, and I'll grant
additional time for people to review the second version at the community.

> 
> 
> > +
> > +
> > +Trusted Reviewers
> > +-----------------
> > +While a maintainer / maintainer-team is expected to be reviewer of last
> > +resort the review load is less onerous when distributed amongst
> > +contributors and or a trusted set of individuals.  This section is
> > +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> > +reviewers that should always be copied on a patch submission, the
> > +trusted reviewers here are individuals contributors can reach out to if
> > +a few 'Resubmit Cadence' intervals have gone by without maintainer
> > +action, or to otherwise consult for advice.  
> 
> This seems redundant with the MAINTAINERS reviewers list.  It seems like
> the role specified in this section is more of an ombudsman or developer
> advocate who can assist with the review and/or accept flow if the
> maintainer is being slow to respond.

Well, on subsystems that have sub-maintainers, there's no way to point to
it at MAINTAINERS file.

Also, not sure about others, but I usually avoid touching at existing
MAINTAINERS file entries. This is a file that everyone touches, so it
has higher chances of conflicts.

Also, at least on media, we have 5 different API sets (digital TV, V4L2, CEC,
media controller, remote controller). Yet, all drivers are stored at the
same place (as a single driver may use multiple APIs).

The reviewers for each API set are different. There isn't a good way
to explain that inside a MAINTANERS file.

> 
> 
> > +
> > +
> > +Time Zone / Office Hours
> > +------------------------
> > +Let contributors know the time of day when one or more maintainers are
> > +usually actively monitoring the mailing list.  
> 
> I would strike "actively monitoring the mailing list".  To me, it should
> be what are the hours of the day that the maintainer might happen to poll
> (or might receive an interrupt) from the appropriate communications
> channels (could be IRC, could be email, etc).
> 
> For my area, I would want to say something like: I tend to be active
> between 17:00 UTC (18:00 UTC when daylight savings) and 25:00 (26:00),
> but often will check for urgent or brief items up until 07:00 (08:00).
> I interact with email via a poll model.  I interact with IRC via a
> pull model and often overlook IRC activity for multiple days).

Frankly, for media, I don't think that working hours makes sense. Media 
(sub-)maintainers are spread around the globe, on different time zones
(in US, Brazil and Europe). We also have several active developers in
Japan, so we may end by having some day reviewers/sub-maintainers from
there.

At max, we can say that we won't warrant to patches on weekends or holidays.

Cheers,
Mauro
Jani Nikula Nov. 16, 2018, 12:44 p.m. UTC | #12
On Thu, 15 Nov 2018, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Thu, 15 Nov 2018, Geert Uytterhoeven wrote:
>
>> Hi Julia,
>>
>> On Thu, Nov 15, 2018 at 6:48 AM Julia Lawall <julia.lawall@lip6.fr> wrote:
>> > How about patch subject lines?  What is the formula that should be used to
>> > transform the name(s) of the affected file(s) into an appropriate suject
>> > line?
>>
>> Automating that may be difficult.
>> I always use "git log --oneline", and try to derive something sane
>> from its output.
>
> Yes, I do likewise.  But there may be some subsystems for which it would
> be possible to come up with a more specific policy.  The advantage of what
> is proposed here is that it is not necessary to come up with a single
> formula that works everywhere.  Even a description in English could be
> helpful.

I quickly cooked up this script to produce the top-5 commit prefixes for
the given files over the arbitrary last 200 commits. It'll give you a
pretty good idea if you're even close.

---
#!/bin/sh
# usage: subject-prefix FILE [...]
# show top 5 subject prefixes for FILEs

git log --format=%s -n 200 -- "$@" |\
	grep -v "^Merge " |\
	sed 's/\(.*\):.*/\1/' |\
	sort | uniq -c | sort -nr | sed 's/ *[0-9]\+ //' |\
	head -n 5
---

Someone who knows perl could turn that into a checkpatch check: See if
the patch subject prefix is one of the top-5 for all files changed by
the patch, and ask the user to double check if it isn't. Or some
heuristics thereof.

BR,
Jani.
Frank Rowand Nov. 16, 2018, 4:47 p.m. UTC | #13
On 11/15/18 4:11 PM, Frank Rowand wrote:
> On 11/14/18 8:53 PM, Dan Williams wrote:

< snip >

>> +
>> +
>> +Time Zone / Office Hours
>> +------------------------
>> +Let contributors know the time of day when one or more maintainers are
>> +usually actively monitoring the mailing list.
> 
> I would strike "actively monitoring the mailing list".  To me, it should
> be what are the hours of the day that the maintainer might happen to poll
> (or might receive an interrupt) from the appropriate communications
> channels (could be IRC, could be email, etc).
> 
> For my area, I would want to say something like: I tend to be active
> between 17:00 UTC (18:00 UTC when daylight savings) and 25:00 (26:00),
> but often will check for urgent or brief items up until 07:00 (08:00).
> I interact with email via a poll model.  I interact with IRC via a
> pull model and often overlook IRC activity for multiple days).

  ^^^^ typo, that should be "poll", not "pull"

> 
> -Frank
> 

>
Joe Perches Nov. 16, 2018, 5:56 p.m. UTC | #14
On Fri, 2018-11-16 at 14:44 +0200, Jani Nikula wrote:
> I quickly cooked up this script to produce the top-5 commit prefixes for
> the given files over the arbitrary last 200 commits. It'll give you a
> pretty good idea if you're even close.
> 
> ---
> #!/bin/sh
> # usage: subject-prefix FILE [...]
> # show top 5 subject prefixes for FILEs
> 
> git log --format=%s -n 200 -- "$@" |\
> 	grep -v "^Merge " |\
> 	sed 's/\(.*\):.*/\1/' |\
> 	sort | uniq -c | sort -nr | sed 's/ *[0-9]\+ //' |\
> 	head -n 5
> ---
> 
> Someone who knows perl could turn that into a checkpatch check: See if
> the patch subject prefix is one of the top-5 for all files changed by
> the patch, and ask the user to double check if it isn't. Or some
> heuristics thereof.

This won't work when a patch contains multiple files
from different paths, or even multiple files from a
single driver.

Perhaps it's better to use a generic mechanism like

	basename $(dirname $filename):

with some exceptions and add an override patch subject
grammar to appropriate various sections of MAINTAINERS.

I also think it's better to use a separate script like
scripts/spdxcheck.py and tie any necessary checkpatch
use to that script.
Dan Williams Nov. 16, 2018, 6:57 p.m. UTC | #15
On Fri, Nov 16, 2018 at 4:04 AM Mauro Carvalho Chehab
<mchehab@kernel.org> wrote:
>
> Em Thu, 15 Nov 2018 16:11:59 -0800
> Frank Rowand <frowand.list@gmail.com> escreveu:
[..]
> > > +Patches or Pull requests
> > > +------------------------
> > > +Some subsystems allow contributors to send pull requests, most require
> > > +mailed patches. State “Patches only”, or “Pull requests accepted”.
> >
> > This may create some confusion if only "Pull requests accepted" is the
> > contents of this section.  My understanding has been that all changes
> > should be available on a mail list for review before being accepted
> > by the maintainer, even if eventually the final version of the series
> > that is accepted is applied by the maintainer as a pull instead of
> > via applying patches.
> >
> > Is my "must appear on a mail list" understanding correct?
>
> I guess this is true on almost all subsystems that accept pull requests.
>
> It could not be true if some subsystem moves to Github/Gitlab.

Yes. Maybe a "Review Forum" section for subsystems that have
transitioned from email to a web-based tool? There's also the
exception of security disclosures, but the expectations for those
patches are already documented.

> > > +Last -rc for new feature submissions
> > > +------------------------------------
> > > +New feature submissions targeting the next merge window should have
> > > +their first posting for consideration before this point. Patches that
> > > +are submitted after this point should be clear that they are targeting
> > > +the NEXT+1 merge window, or should come with sufficient justification
> > > +why they should be considered on an expedited schedule. A general
> > > +guideline is to set expectation with contributors that new feature
> > > +submissions should appear before -rc5. The answer may be different for
> > > +'Core:' files, include a second entry prefixed with 'Core:' if so.
> >
> > I would expect the -rc to vary based on the patch series size, complexity,
> > risk, number of revisions that will occur, how controversial, number of
> > -rc releases before Linus chooses to release, etc.  You would need a
> > crystal ball to predict much of this.  I could see being able to provide
> > a good estimate of this value for a relatively simple patch.
>
> That's only partially true. I mean, the usual flux is to go up to -rc7.
> After that, the final version is usually merged (except when something
> goes wrong).
>
> Anyway, I guess nobody would complain if the maintainers do an exception
> here and accept more patches when they know that the last rc version
> won't be -rc7, but, instead, -rc8 or -rc9.
>
> This is a general ruleset that describes the usual behavior, telling the
> developers the expected behavior. If the maintainers can do more on some
> particular development cycle, it should be fine.

Yes, and perhaps I should clarify that this is the point at which a
maintainer will start to push back in the typical case, and indicate
to a contributor that they are standing in exceptional territory.
Similar to how later in the -rc series patches get increasing
scrutiny.

> > > +Last -rc to merge features
> > > +--------------------------
> > > +Indicate to contributors the point at which an as yet un-applied patch
> > > +set will need to wait for the NEXT+1 merge window. Of course there is no
> > > +obligation to ever except any given patchset, but if the review has not
> > > +concluded by this point the expectation the contributor should wait and
> > > +resubmit for the following merge window. The answer may be different for
> > > +'Core:' files, include a second entry prefixed with 'Core:' if so.
> >
> > Same comment as for the previous section, with the additional complexity
> > that each unique patch series should bake in -next.

The intent is to try to leave all "should" or prescriptive statements
out of the profile. I'm looking to target other parts of the handbook
to carry advice for integrating with -next and suggested soak times.

> > > +Non-author Ack / Review Tags Required
> > > +-------------------------------------
> >
> > The title of this section seems a bit different than the description
> > below.  A more aligned title would be something like "Maintainer
> > self-commit review requirements".

This is intended to go beyond maintainer self-commits. Consider a pull
request that contains commits without non-author tags.

> > > +Let contributors and other maintainers know whether they can expect to
> > > +see the maintainer self-commit patches without 3rd-party review. Some
> > > +subsystem developer communities are so small as to make this requirement
> > > +impractical. Others may have been bootstrapped by a submission of
> > > +self-reviewed code at the outset, but have since moved to a
> > > +non-author review-required stance. This section sets expectations on the
> > > +code-review economics in the subsystem. For example, can a contributor
> > > +trade review of a maintainer's, or other contributor's patches in
> > > +exchange for consideration of their own.
> >
> > Then another section (which is the one I expected when I slightly
> > mis-read the section title) would be: Review Tags Requirements",
> > which would discuss what type of review is expected, encouraged,
> > or required.

Per the clarification above, I think the whole thing should be called
"Review Tags Requirement".

> > > +Test Suite
> > > +----------
> > > +Indicate the test suite all patches are expected to pass before being
> > > +submitted for inclusion consideration.
> > > +
> > > +
> > > +Resubmit Cadence
> > > +----------------
> > > +Define a rate at which a contributor should wait to resubmit a patchset
> > > +that has not yet received comments. A general guideline is to try to
> > > +meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
> > > +a patch set.
> >
> > Or ping instead of resubmitting?  If you resubmit the same series with
> > an unchanged version then comments could be split across multiple
> > email threads.  If you resubmit the same series with a new version
> > number, the same comment split can occur plus it can be confusing
> > that two versions have the same contents.  I suspect that there may be
> > some maintainers who do prefer a resubmit, but that is just wild
> > conjecture on my part.
>
> That depends on how maintainers handle patches. Those subsystems that
> use patchwork (like media) don't really require anyone to resubmit
> patches. They're all stored there at patchwork database.
>
> My workflow is that, if I receive two patches from the same person with
> the same subject, I simply mark the first one as obsoleted, as usually
> the second one is a new version, and reviewers should need some
> time to handle it.
>
> In other words, re-sending a patch may actually delay its handling, as
> the second version will be later on my input queue, and I'll grant
> additional time for people to review the second version at the community.

Ok, this sounds like a separate policy. How often to follow-up
separated from resend the full series vs send a ping mail.

> > > +Trusted Reviewers
> > > +-----------------
> > > +While a maintainer / maintainer-team is expected to be reviewer of last
> > > +resort the review load is less onerous when distributed amongst
> > > +contributors and or a trusted set of individuals.  This section is
> > > +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> > > +reviewers that should always be copied on a patch submission, the
> > > +trusted reviewers here are individuals contributors can reach out to if
> > > +a few 'Resubmit Cadence' intervals have gone by without maintainer
> > > +action, or to otherwise consult for advice.
> >
> > This seems redundant with the MAINTAINERS reviewers list.  It seems like
> > the role specified in this section is more of an ombudsman or developer
> > advocate who can assist with the review and/or accept flow if the
> > maintainer is being slow to respond.
>
> Well, on subsystems that have sub-maintainers, there's no way to point to
> it at MAINTAINERS file.
>
> Also, not sure about others, but I usually avoid touching at existing
> MAINTAINERS file entries. This is a file that everyone touches, so it
> has higher chances of conflicts.
>
> Also, at least on media, we have 5 different API sets (digital TV, V4L2, CEC,
> media controller, remote controller). Yet, all drivers are stored at the
> same place (as a single driver may use multiple APIs).
>
> The reviewers for each API set are different. There isn't a good way
> to explain that inside a MAINTANERS file.

Would it be worthwhile to have separate Subsystem Profiles for those
API reviewers? If they end up merging patches and sending them
upstream might we need a hierarchy of profiles for each hop along the
upstream merge path?

> > > +Time Zone / Office Hours
> > > +------------------------
> > > +Let contributors know the time of day when one or more maintainers are
> > > +usually actively monitoring the mailing list.
> >
> > I would strike "actively monitoring the mailing list".  To me, it should
> > be what are the hours of the day that the maintainer might happen to poll
> > (or might receive an interrupt) from the appropriate communications
> > channels (could be IRC, could be email, etc).

Yes, makes sense.

> > For my area, I would want to say something like: I tend to be active
> > between 17:00 UTC (18:00 UTC when daylight savings) and 25:00 (26:00),
> > but often will check for urgent or brief items up until 07:00 (08:00).
> > I interact with email via a poll model.  I interact with IRC via a
> > pull model and often overlook IRC activity for multiple days).
>
> Frankly, for media, I don't think that working hours makes sense. Media
> (sub-)maintainers are spread around the globe, on different time zones
> (in US, Brazil and Europe). We also have several active developers in
> Japan, so we may end by having some day reviewers/sub-maintainers from
> there.

For that case just say:

"the sun never sets on the media subsystem" ;-)

> At max, we can say that we won't warrant to patches on weekends or holidays.

Yeah, maybe:

"outside of weekends or holidays there's usually a maintainer or
reviewer monitoring the mailing list"
Randy Dunlap Nov. 16, 2018, 11:28 p.m. UTC | #16
On 11/15/18 7:44 AM, Mauro Carvalho Chehab wrote:
> 
> Anyway, RFC patch follows.
> 
> -
> 
> [PATCH] [RFC] Add a system profile description for media subsystem
> 
> This RFC aligns with current Dan's proposal for having subsystem
> specific ruleset stored at the Kernel tree.
> 
> On this initial RFC, I opted to not add the reviewers e-mail
> (adding just a "<>") as a boilerplate. If we decide keeping emails
> there, I'll add them.

Hi-
Here are my comments.

Hopefully the email addresses will be added.  Just having names is a
half-answer for contact info.

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> 
> diff --git a/Documentation/media/subsystem-profile.rst b/Documentation/media/subsystem-profile.rst
> new file mode 100644
> index 000000000000..7a5d6f691d05
> --- /dev/null
> +++ b/Documentation/media/subsystem-profile.rst
> @@ -0,0 +1,186 @@
> +Media Subsystem Profile
> +=======================
> +
> +Overview
> +--------
> +
> +The media subsystem cover support for a variety of devices: stream

                       covers

> +capture, analog and digital TV, cameras, remote controllers, HDMI CEC
> +and media pipeline control.
> +
> +Both our userspace and Kernel APIs are documented and should be kept in
> +sync with the API changes. It means that all patches that add new
> +features to the subsystem should also bring changes to the corresponding
> +API files.
> +
> +Also, patches for device drivers that changes the Open Firmware/Device
> +Tree bindings should be reviewed by the Device Tree maintainers.
> +
> +Due to the size and wide scope of the media subsystem, our
> +maintainership model is to have sub-maintainers that have a broad
> +knowledge of an specific aspect of the subsystem. It is a

             of a specific

> +sub-maintainers task to review the patches, providing feedback to users
> +if the patches are following the subsystem rules and are properly using
> +the media internal and external APIs.
> +
> +We have a set of compliance tools at https://git.linuxtv.org/v4l-utils.git/
> +that should be used in order to check if the drivers are properly
> +implementing the media APIs.
> +
> +Patches for the media subsystem should be sent to the media mailing list
> +at linux-media@vger.kernel.org as plain text only e-mail. emails with

                               e-mail or email?  Be consistent. (more below)

> +HTML will be automacially rejected by the mail server.

                automatically

> +
> +Our workflow is heavily based on Patchwork, meaning that, once a patch
> +is submitted, it should appear at:
> +
> +   - https://patchwork.linuxtv.org/project/linux-media/list/
> +
> +If it doesn't automatically appear there after a few minutes, then
> +probably something got wrong on your submission. Please check if the
> +email is in plain text only and if your emailer is not mangling with

email

> +whitespaces before complaining or submit it again.
> +
> +Core
> +----
> +
> +Documentation
> ++++++++++++++
> +
> +F: Documentation/media
> +
> +Kernelspace API headers
> ++++++++++++++++++++++++
> +
> +F: include/media/*.h
> +
> +Digital TV Core
> ++++++++++++++++
> +
> +F: drivers/media/dvb-core
> +
> +HDMI CEC Core
> ++++++++++++++
> +
> +F: drivers/media/cec
> +
> +Media Controller Core
> ++++++++++++++++++++++
> +
> +F: drivers/media/media-\*.[ch]
> +
> +Remote Controller Core
> +++++++++++++++++++++++
> +
> +F: drivers/media/rc/rc-core-priv.h
> +F: drivers/media/rc/rc-ir-raw.c
> +F: drivers/media/rc/rc-main.c
> +F: drivers/media/rc/ir\*-decoder.c
> +F: drivers/media/rc/lirc_dev.c
> +
> +Video4linux Core
> +++++++++++++++++
> +
> +F: drivers/media/v4l2-core
> +
> +Patches or Pull requests
> +------------------------
> +
> +All patches should be submitted via e-mail for review. We use

and e-mail

> +pull requests on our workflow between sub-maintainers and the
> +maintainer.
> +
> +
> +Last day for new feature submissions
> +------------------------------------
> +
> +Before -rc5
> +
> +
> +Last day to merge features
> +--------------------------
> +
> +Before -rc7
> +
> +
> +Non-author Ack / Review Tags Required
> +-------------------------------------
> +
> +Not required, but desirable
> +
> +
> +Test Suite
> +----------
> +
> +Use the several *-compliance tools that are part of the v4l-utils
> +package.
> +
> +
> +Trusted Reviewers
> +-----------------
> +
> +Sub-maintainers
> ++++++++++++++++
> +
> +At the media subsystem, we have a group of senior developers that are
> +responsible for doing the code reviews at the drivers (called
> +sub-maintainers), and another senior developer responsible for the
> +subsystem as a hole. For core changes, whenever possible, multiple

             as a whole.

> +media (sub-)maintainers do the review.
> +
> +The sub-maintainers work on specific areas of the subsystem, as
> +described below:
> +
> +- Sensor drivers
> +
> +  R: Sakari Ailus <>
> +
> +- V4L2 drivers
> +
> +  R: Hans Verkuil <>
> +
> +- Media controller drivers
> +
> +  R: Laurent Pinchart <>
> +
> +- HDMI CEC
> +
> +- Remote Controllers
> +
> +  R: Sean Young <>
> +
> +- Digital TV
> +
> +  R: Michael Krufky <>
> +  R: Sean Young <>
> +
> +
> +Resubmit Cadence
> +----------------
> +
> +Provided that your patch is at https://patchwork.linuxtv.org, it should
> +be sooner or later handled, so you don't need to re-submit a patch.

Resubmit or re-submit?  Be consistent.

> +
> +Please notice that the media subsystem is a high traffic one, so it
> +could take a while for us to be able to review your patches. Feel free
> +to ping if you don't get a feedback on a couple of weeks.

                                       in a

> +
> +Time Zone / Office Hours
> +------------------------
> +
> +Media developers are distributed all around the globe. So, don't assume
> +that we're on your time zone. We usually don't work on local holidays or
> +at weekends. Please also notice that, during the Kernel merge window,
> +we're usually busy ensuring that everything goes smoothly, meaning that
> +we usually have a lot of patches waiting for review just after that. So
> +you should expect a higher delay during the merge window and one week
> +before/after it.
> +
> +
> +Checkpatch / Style cleanups
> +---------------------------
> +
> +Standalone style-cleanups are welcome, but they should be grouped per
> +directory. So, for example, if you're doing a cleanup at drivers
> +under drivers/media, please send a single patch for all drivers under
> +drivers/media/pci, another one for drivers/media/usb and so on.


> Cheers.- 
~Randy
Hans Verkuil Nov. 17, 2018, 11:57 a.m. UTC | #17
On 11/15/2018 04:44 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 14 Nov 2018 20:53:25 -0800
> Dan Williams <dan.j.williams@intel.com> escreveu:
> 
>> As presented at the 2018 Linux Plumbers conference [1], the Subsystem
>> Profile is proposed as a way to reduce friction between committers and
>> maintainers and perhaps encourage conversations amongst maintainers
>> about best practice policies.
>>
>> The profile contains short answers to some of the common policy
>> questions a contributor might have, or that a maintainer might consider
>> formalizing. The current list of maintenance policies is:
>>
>> Overview: General introduction to maintaining the subsystem
>> Core: List of source files considered core
>> Leaf: List of source files that consume core functionality
>> Patches or Pull requests: Simple statement of expected submission format
>> Last -rc for new feature submissions: Expected lead time for submissions
>> Last -rc to merge features: Deadline for merge decisions
>> Non-author Ack / Review Tags Required: Patch review economics
>> Test Suite: Pass this suite before requesting inclusion
>> Resubmit Cadence: When to ping the maintainer
>> Trusted Reviewers: Help for triaging patches
>> Time Zone / Office Hours: When might a maintainer be available
>> Checkpatch / Style Cleanups: Policy on pure cleanup patches
>> Off-list review: Request for review gates
>> TODO: Potential development tasks up for grabs, or active focus areas
>>
>> The goal of the Subsystem Profile is to set expectations for
>> contributors and interim or replacement maintainers for a subsystem.
>>
>> See Documentation/maintainer/subsystem-profile.rst for more details, and
>> a follow-on example profile for the libnvdimm subsystem.
>>
>> [1]: https://linuxplumbersconf.org/event/2/contributions/59/
>>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>> Cc: Steve French <stfrench@microsoft.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Tobin C. Harding <me@tobin.cc>
>> Cc: Olof Johansson <olof@lixom.net>
>> Cc: Martin K. Petersen <martin.petersen@oracle.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Joe Perches <joe@perches.com>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Ok, I sort of followed the proposed model, adding a documentation for
> the media subsystem using the template.
> 
> I'm pretty sure this is incomplete and more work would be needed.
> So, for now, this is just an example.
> 
> Also, I didn't test building it on Sphinx (nor added any reference for it
> at MAINTAINERS), as my main goal here was to see how the model would fit
> for us.
> 
> I noticed a few issues there:
> 
> 1) Describing the "core" files. The media subsystem is complex: depending on
> the device type, we have different APIs and different cores. So, our core
> is actually a set of different cores. I ended by adding sub-sections.
> 
> Also, several things were not described there. For example, we store
> DVB frontend drivers on an specific directory. We have drivers there organized
> by bus type. So, one directory for I2C, one for USB, one for PCI, ...
> 
> It should probably make sense to be able to tell about that.
> 
> So, I would actually prefer to have, at the profile, a "files" section,
> instead of "core".
> 
> 2) As I said before, on media, we have sub-maintainers. Not sure how to
> describe them on this template.
> 
> 3) I noticed one big missing thing there: in the case of media, we are
> responsible also to maintaining media staging drivers, that goes at
> drivers/staging/media. IMO, it would be important to be able to describe
> who maintains staging stuff - e. g. if the subsystem maintainers prefer
> to let staging up to staging maintainers or if they'll maintain themselves.
> 
> In the latter, it probably makes sense to describe any specific thing
> related to it (where staging will be at the tree, are there any special
> rules for them?)
> 
> Anyway, RFC patch follows.
> 
> -
> 
> [PATCH] [RFC] Add a system profile description for media subsystem
> 
> This RFC aligns with current Dan's proposal for having subsystem
> specific ruleset stored at the Kernel tree.
> 
> On this initial RFC, I opted to not add the reviewers e-mail
> (adding just a "<>") as a boilerplate. If we decide keeping emails
> there, I'll add them.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> 
> diff --git a/Documentation/media/subsystem-profile.rst b/Documentation/media/subsystem-profile.rst
> new file mode 100644
> index 000000000000..7a5d6f691d05
> --- /dev/null
> +++ b/Documentation/media/subsystem-profile.rst
> @@ -0,0 +1,186 @@
> +Media Subsystem Profile
> +=======================
> +
> +Overview
> +--------
> +
> +The media subsystem cover support for a variety of devices: stream
> +capture, analog and digital TV, cameras, remote controllers, HDMI CEC
> +and media pipeline control.

And radio tuners and modulators, including support for RDS (Radio Data System),
and also software defined radio tuners and modulators.

We also support video codecs (very important to mention this!) and video
memory-to-memory processing (deinterlacers, scalers, etc.).

There is also support for touch devices and VBI (Vertical Blanking Interface).

> +
> +Both our userspace and Kernel APIs are documented and should be kept in
> +sync with the API changes. It means that all patches that add new
> +features to the subsystem should also bring changes to the corresponding
> +API files.
> +
> +Also, patches for device drivers that changes the Open Firmware/Device
> +Tree bindings should be reviewed by the Device Tree maintainers.
> +
> +Due to the size and wide scope of the media subsystem, our
> +maintainership model is to have sub-maintainers that have a broad
> +knowledge of an specific aspect of the subsystem. It is a
> +sub-maintainers task to review the patches, providing feedback to users
> +if the patches are following the subsystem rules and are properly using
> +the media internal and external APIs.
> +
> +We have a set of compliance tools at https://git.linuxtv.org/v4l-utils.git/
> +that should be used in order to check if the drivers are properly
> +implementing the media APIs.

New V4L2 and HDMI CEC drivers must pass their corresponding compliance tests
before they are accepted into the kernel.

Adding new public V4L2 or HDMI CEC APIs will also require adding new compliance
tests to ensure that the new APIs are tested.

> +
> +Patches for the media subsystem should be sent to the media mailing list
> +at linux-media@vger.kernel.org as plain text only e-mail. emails with
> +HTML will be automacially rejected by the mail server.
> +
> +Our workflow is heavily based on Patchwork, meaning that, once a patch
> +is submitted, it should appear at:
> +
> +   - https://patchwork.linuxtv.org/project/linux-media/list/
> +
> +If it doesn't automatically appear there after a few minutes, then
> +probably something got wrong on your submission. Please check if the
> +email is in plain text only and if your emailer is not mangling with
> +whitespaces before complaining or submit it again.
> +
> +Core
> +----
> +
> +Documentation
> ++++++++++++++
> +
> +F: Documentation/media
> +
> +Kernelspace API headers
> ++++++++++++++++++++++++
> +
> +F: include/media/*.h
> +
> +Digital TV Core
> ++++++++++++++++
> +
> +F: drivers/media/dvb-core
> +
> +HDMI CEC Core
> ++++++++++++++
> +
> +F: drivers/media/cec
> +
> +Media Controller Core
> ++++++++++++++++++++++
> +
> +F: drivers/media/media-\*.[ch]
> +
> +Remote Controller Core
> +++++++++++++++++++++++
> +
> +F: drivers/media/rc/rc-core-priv.h
> +F: drivers/media/rc/rc-ir-raw.c
> +F: drivers/media/rc/rc-main.c
> +F: drivers/media/rc/ir\*-decoder.c
> +F: drivers/media/rc/lirc_dev.c
> +
> +Video4linux Core
> +++++++++++++++++
> +
> +F: drivers/media/v4l2-core

We should mention vb2 here as well:

F: drivers/media/common/videobuf2

It's a critical part of the V4L2/DVB core infrastructure.

> +
> +Patches or Pull requests
> +------------------------
> +
> +All patches should be submitted via e-mail for review. We use
> +pull requests on our workflow between sub-maintainers and the
> +maintainer.
> +
> +
> +Last day for new feature submissions
> +------------------------------------
> +
> +Before -rc5
> +
> +
> +Last day to merge features
> +--------------------------
> +
> +Before -rc7
> +
> +
> +Non-author Ack / Review Tags Required
> +-------------------------------------
> +
> +Not required, but desirable
> +
> +
> +Test Suite
> +----------
> +
> +Use the several *-compliance tools that are part of the v4l-utils
> +package.
> +
> +
> +Trusted Reviewers
> +-----------------
> +
> +Sub-maintainers
> ++++++++++++++++
> +
> +At the media subsystem, we have a group of senior developers that are
> +responsible for doing the code reviews at the drivers (called
> +sub-maintainers), and another senior developer responsible for the
> +subsystem as a hole. For core changes, whenever possible, multiple
> +media (sub-)maintainers do the review.
> +
> +The sub-maintainers work on specific areas of the subsystem, as
> +described below:
> +
> +- Sensor drivers
> +
> +  R: Sakari Ailus <>
> +
> +- V4L2 drivers
> +
> +  R: Hans Verkuil <>
> +
> +- Media controller drivers
> +
> +  R: Laurent Pinchart <>
> +
> +- HDMI CEC

That's me as well :-)

> +
> +- Remote Controllers
> +
> +  R: Sean Young <>
> +
> +- Digital TV
> +
> +  R: Michael Krufky <>
> +  R: Sean Young <>
> +
> +
> +Resubmit Cadence
> +----------------
> +
> +Provided that your patch is at https://patchwork.linuxtv.org, it should
> +be sooner or later handled, so you don't need to re-submit a patch.
> +
> +Please notice that the media subsystem is a high traffic one, so it
> +could take a while for us to be able to review your patches. Feel free
> +to ping if you don't get a feedback on a couple of weeks.

Hmm, I would prefer a ping in a week for high-prio patches. Otherwise wait
2-3 weeks.

> +
> +Time Zone / Office Hours
> +------------------------
> +
> +Media developers are distributed all around the globe. So, don't assume
> +that we're on your time zone. We usually don't work on local holidays or
> +at weekends. Please also notice that, during the Kernel merge window,
> +we're usually busy ensuring that everything goes smoothly, meaning that
> +we usually have a lot of patches waiting for review just after that. So
> +you should expect a higher delay during the merge window and one week
> +before/after it.
> +
> +
> +Checkpatch / Style cleanups
> +---------------------------
> +
> +Standalone style-cleanups are welcome, but they should be grouped per
> +directory. So, for example, if you're doing a cleanup at drivers
> +under drivers/media, please send a single patch for all drivers under
> +drivers/media/pci, another one for drivers/media/usb and so on.

Regards,

	Hans

> 
> 
> 
> 
> 
> Cheers,
> Mauro
>
Rob Herring Nov. 17, 2018, 2:12 p.m. UTC | #18
On Fri, Nov 16, 2018 at 11:57 AM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2018-11-16 at 14:44 +0200, Jani Nikula wrote:
> > I quickly cooked up this script to produce the top-5 commit prefixes for
> > the given files over the arbitrary last 200 commits. It'll give you a
> > pretty good idea if you're even close.
> >
> > ---
> > #!/bin/sh
> > # usage: subject-prefix FILE [...]
> > # show top 5 subject prefixes for FILEs
> >
> > git log --format=%s -n 200 -- "$@" |\
> >       grep -v "^Merge " |\

--no-merges in git log can replace this line.

> >       sed 's/\(.*\):.*/\1/' |\
> >       sort | uniq -c | sort -nr | sed 's/ *[0-9]\+ //' |\
> >       head -n 5
> > ---
> >
> > Someone who knows perl could turn that into a checkpatch check: See if
> > the patch subject prefix is one of the top-5 for all files changed by
> > the patch, and ask the user to double check if it isn't. Or some
> > heuristics thereof.
>
> This won't work when a patch contains multiple files
> from different paths, or even multiple files from a
> single driver.

Different paths is often, but not always a sign that patches may need
to be split up. Maybe that is something checkpatch should point out.

> Perhaps it's better to use a generic mechanism like
>
>         basename $(dirname $filename):
>
> with some exceptions and add an override patch subject
> grammar to appropriate various sections of MAINTAINERS.

Perhaps just use the script as a starting point to populate
MAINTAINERS as it may never be that accurate.

> I also think it's better to use a separate script like
> scripts/spdxcheck.py and tie any necessary checkpatch
> use to that script.

Yes, checkpatch is getting pretty unwieldy.

Rob
Julia Lawall Nov. 17, 2018, 5:03 p.m. UTC | #19
On Sat, 17 Nov 2018, Rob Herring wrote:

> On Fri, Nov 16, 2018 at 11:57 AM Joe Perches <joe@perches.com> wrote:
> >
> > On Fri, 2018-11-16 at 14:44 +0200, Jani Nikula wrote:
> > > I quickly cooked up this script to produce the top-5 commit prefixes for
> > > the given files over the arbitrary last 200 commits. It'll give you a
> > > pretty good idea if you're even close.
> > >
> > > ---
> > > #!/bin/sh
> > > # usage: subject-prefix FILE [...]
> > > # show top 5 subject prefixes for FILEs
> > >
> > > git log --format=%s -n 200 -- "$@" |\
> > >       grep -v "^Merge " |\
>
> --no-merges in git log can replace this line.
>
> > >       sed 's/\(.*\):.*/\1/' |\
> > >       sort | uniq -c | sort -nr | sed 's/ *[0-9]\+ //' |\
> > >       head -n 5
> > > ---
> > >
> > > Someone who knows perl could turn that into a checkpatch check: See if
> > > the patch subject prefix is one of the top-5 for all files changed by
> > > the patch, and ask the user to double check if it isn't. Or some
> > > heuristics thereof.
> >
> > This won't work when a patch contains multiple files
> > from different paths, or even multiple files from a
> > single driver.
>
> Different paths is often, but not always a sign that patches may need
> to be split up. Maybe that is something checkpatch should point out.

Between v4.0 and v4.19, 18% of commits touch multiple .c files.  35% of
commits touch multiple files in general.

julia

>
> > Perhaps it's better to use a generic mechanism like
> >
> >         basename $(dirname $filename):
> >
> > with some exceptions and add an override patch subject
> > grammar to appropriate various sections of MAINTAINERS.
>
> Perhaps just use the script as a starting point to populate
> MAINTAINERS as it may never be that accurate.
>
> > I also think it's better to use a separate script like
> > scripts/spdxcheck.py and tie any necessary checkpatch
> > use to that script.
>
> Yes, checkpatch is getting pretty unwieldy.
>
> Rob
>
Mauro Carvalho Chehab Nov. 18, 2018, 12:58 p.m. UTC | #20
Em Fri, 16 Nov 2018 10:57:14 -0800
Dan Williams <dan.j.williams@intel.com> escreveu:

> On Fri, Nov 16, 2018 at 4:04 AM Mauro Carvalho Chehab
> <mchehab@kernel.org> wrote:
> >
> > Em Thu, 15 Nov 2018 16:11:59 -0800
> > Frank Rowand <frowand.list@gmail.com> escreveu:  
> [..]
> > > > +Patches or Pull requests
> > > > +------------------------
> > > > +Some subsystems allow contributors to send pull requests, most require
> > > > +mailed patches. State “Patches only”, or “Pull requests accepted”.  
> > >
> > > This may create some confusion if only "Pull requests accepted" is the
> > > contents of this section.  My understanding has been that all changes
> > > should be available on a mail list for review before being accepted
> > > by the maintainer, even if eventually the final version of the series
> > > that is accepted is applied by the maintainer as a pull instead of
> > > via applying patches.
> > >
> > > Is my "must appear on a mail list" understanding correct?  
> >
> > I guess this is true on almost all subsystems that accept pull requests.
> >
> > It could not be true if some subsystem moves to Github/Gitlab.  
> 
> Yes. Maybe a "Review Forum" section for subsystems that have
> transitioned from email to a web-based tool? There's also the
> exception of security disclosures, but the expectations for those
> patches are already documented.

Maybe. I would postpone adding a section like that until some
subsystem maintainer that actually changed to Github/Gitlab
would submit his subsystem profile.

> 
> > > > +Last -rc for new feature submissions
> > > > +------------------------------------
> > > > +New feature submissions targeting the next merge window should have
> > > > +their first posting for consideration before this point. Patches that
> > > > +are submitted after this point should be clear that they are targeting
> > > > +the NEXT+1 merge window, or should come with sufficient justification
> > > > +why they should be considered on an expedited schedule. A general
> > > > +guideline is to set expectation with contributors that new feature
> > > > +submissions should appear before -rc5. The answer may be different for
> > > > +'Core:' files, include a second entry prefixed with 'Core:' if so.  
> > >
> > > I would expect the -rc to vary based on the patch series size, complexity,
> > > risk, number of revisions that will occur, how controversial, number of
> > > -rc releases before Linus chooses to release, etc.  You would need a
> > > crystal ball to predict much of this.  I could see being able to provide
> > > a good estimate of this value for a relatively simple patch.  
> >
> > That's only partially true. I mean, the usual flux is to go up to -rc7.
> > After that, the final version is usually merged (except when something
> > goes wrong).
> >
> > Anyway, I guess nobody would complain if the maintainers do an exception
> > here and accept more patches when they know that the last rc version
> > won't be -rc7, but, instead, -rc8 or -rc9.
> >
> > This is a general ruleset that describes the usual behavior, telling the
> > developers the expected behavior. If the maintainers can do more on some
> > particular development cycle, it should be fine.  
> 
> Yes, and perhaps I should clarify that this is the point at which a
> maintainer will start to push back in the typical case, and indicate
> to a contributor that they are standing in exceptional territory.
> Similar to how later in the -rc series patches get increasing
> scrutiny.

Makes sense. There's one issue, though.

I don't expect developers to read the profile template, as this is a
material for the maintainer themselves. Developers should likely read 
just the specific subsystem profile for the patches that will be submitted.

So, either each subsystem profile should have a reference to the
profile template, or need to copy some "invariant" texts (with would be
really painful to maintain).

> 
> > > > +Last -rc to merge features
> > > > +--------------------------
> > > > +Indicate to contributors the point at which an as yet un-applied patch
> > > > +set will need to wait for the NEXT+1 merge window. Of course there is no
> > > > +obligation to ever except any given patchset, but if the review has not
> > > > +concluded by this point the expectation the contributor should wait and
> > > > +resubmit for the following merge window. The answer may be different for
> > > > +'Core:' files, include a second entry prefixed with 'Core:' if so.  
> > >
> > > Same comment as for the previous section, with the additional complexity
> > > that each unique patch series should bake in -next.  
> 
> The intent is to try to leave all "should" or prescriptive statements
> out of the profile. I'm looking to target other parts of the handbook
> to carry advice for integrating with -next and suggested soak times.

Makes sense. Again, it probably makes sense to have those parts
referenced on each profile.

> > > > +Non-author Ack / Review Tags Required
> > > > +-------------------------------------  
> > >
> > > The title of this section seems a bit different than the description
> > > below.  A more aligned title would be something like "Maintainer
> > > self-commit review requirements".  
> 
> This is intended to go beyond maintainer self-commits. Consider a pull
> request that contains commits without non-author tags.
> 
> > > > +Let contributors and other maintainers know whether they can expect to
> > > > +see the maintainer self-commit patches without 3rd-party review. Some
> > > > +subsystem developer communities are so small as to make this requirement
> > > > +impractical. Others may have been bootstrapped by a submission of
> > > > +self-reviewed code at the outset, but have since moved to a
> > > > +non-author review-required stance. This section sets expectations on the
> > > > +code-review economics in the subsystem. For example, can a contributor
> > > > +trade review of a maintainer's, or other contributor's patches in
> > > > +exchange for consideration of their own.  
> > >
> > > Then another section (which is the one I expected when I slightly
> > > mis-read the section title) would be: Review Tags Requirements",
> > > which would discuss what type of review is expected, encouraged,
> > > or required.  
> 
> Per the clarification above, I think the whole thing should be called
> "Review Tags Requirement".

Agreed.

> > > > +Test Suite
> > > > +----------
> > > > +Indicate the test suite all patches are expected to pass before being
> > > > +submitted for inclusion consideration.
> > > > +
> > > > +
> > > > +Resubmit Cadence
> > > > +----------------
> > > > +Define a rate at which a contributor should wait to resubmit a patchset
> > > > +that has not yet received comments. A general guideline is to try to
> > > > +meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
> > > > +a patch set.  
> > >
> > > Or ping instead of resubmitting?  If you resubmit the same series with
> > > an unchanged version then comments could be split across multiple
> > > email threads.  If you resubmit the same series with a new version
> > > number, the same comment split can occur plus it can be confusing
> > > that two versions have the same contents.  I suspect that there may be
> > > some maintainers who do prefer a resubmit, but that is just wild
> > > conjecture on my part.  
> >
> > That depends on how maintainers handle patches. Those subsystems that
> > use patchwork (like media) don't really require anyone to resubmit
> > patches. They're all stored there at patchwork database.
> >
> > My workflow is that, if I receive two patches from the same person with
> > the same subject, I simply mark the first one as obsoleted, as usually
> > the second one is a new version, and reviewers should need some
> > time to handle it.
> >
> > In other words, re-sending a patch may actually delay its handling, as
> > the second version will be later on my input queue, and I'll grant
> > additional time for people to review the second version at the community.  
> 
> Ok, this sounds like a separate policy. How often to follow-up
> separated from resend the full series vs send a ping mail.
> 
> > > > +Trusted Reviewers
> > > > +-----------------
> > > > +While a maintainer / maintainer-team is expected to be reviewer of last
> > > > +resort the review load is less onerous when distributed amongst
> > > > +contributors and or a trusted set of individuals.  This section is
> > > > +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> > > > +reviewers that should always be copied on a patch submission, the
> > > > +trusted reviewers here are individuals contributors can reach out to if
> > > > +a few 'Resubmit Cadence' intervals have gone by without maintainer
> > > > +action, or to otherwise consult for advice.  
> > >
> > > This seems redundant with the MAINTAINERS reviewers list.  It seems like
> > > the role specified in this section is more of an ombudsman or developer
> > > advocate who can assist with the review and/or accept flow if the
> > > maintainer is being slow to respond.  
> >
> > Well, on subsystems that have sub-maintainers, there's no way to point to
> > it at MAINTAINERS file.
> >
> > Also, not sure about others, but I usually avoid touching at existing
> > MAINTAINERS file entries. This is a file that everyone touches, so it
> > has higher chances of conflicts.
> >
> > Also, at least on media, we have 5 different API sets (digital TV, V4L2, CEC,
> > media controller, remote controller). Yet, all drivers are stored at the
> > same place (as a single driver may use multiple APIs).
> >
> > The reviewers for each API set are different. There isn't a good way
> > to explain that inside a MAINTANERS file.  
> 
> Would it be worthwhile to have separate Subsystem Profiles for those
> API reviewers? If they end up merging patches and sending them
> upstream might we need a hierarchy of profiles for each hop along the
> upstream merge path?

I guess having hierarchical profiles will make it very confusing.
The point is: inside a subsystem, the same ruleset usually applies to
everything.

In the case of media, it is not uncommon to have patches that require
multiple APIs. Consider, for example, a SoC used on a TV box. The driver
itself should be placed at drivers/media/platform/, but it will end by
being a bunch of sub-drivers that together will add support for V4L, 
Digital TV, remote controller, CEC and codecs, and need to be controlled
via the media controller API. It may even have camera sensors.

On other words, all media APIs will be used (after having it fully
sent upstream).

In practice, drivers for complex hardware like that is submitted in
parts. For example, one SoC vendor started sending us the remote
controller driver (as it would be the simplest one).

The only part of the policy that changes, depending of what API
is involved, is the one that will do the review. 

As the driver itself will be at the same place, no matter what APIs
are used, get_maintainers.pl is not capable of identifying who are 
the reviewers based "F:" tags[1].

[1] It could be possible to teach get_maintainers to better hint it,
by making it look who are the reviewers for the headers that are 
included.

> 
> > > > +Time Zone / Office Hours
> > > > +------------------------
> > > > +Let contributors know the time of day when one or more maintainers are
> > > > +usually actively monitoring the mailing list.  
> > >
> > > I would strike "actively monitoring the mailing list".  To me, it should
> > > be what are the hours of the day that the maintainer might happen to poll
> > > (or might receive an interrupt) from the appropriate communications
> > > channels (could be IRC, could be email, etc).  
> 
> Yes, makes sense.
> 
> > > For my area, I would want to say something like: I tend to be active
> > > between 17:00 UTC (18:00 UTC when daylight savings) and 25:00 (26:00),
> > > but often will check for urgent or brief items up until 07:00 (08:00).
> > > I interact with email via a poll model.  I interact with IRC via a
> > > pull model and often overlook IRC activity for multiple days).  
> >
> > Frankly, for media, I don't think that working hours makes sense. Media
> > (sub-)maintainers are spread around the globe, on different time zones
> > (in US, Brazil and Europe). We also have several active developers in
> > Japan, so we may end by having some day reviewers/sub-maintainers from
> > there.  
> 
> For that case just say:
> 
> "the sun never sets on the media subsystem" ;-)

:-)

> 
> > At max, we can say that we won't warrant to patches on weekends or holidays.  
> 
> Yeah, maybe:
> 
> "outside of weekends or holidays there's usually a maintainer or
> reviewer monitoring the mailing list"

Well, 24/7, there is always patchwork monitoring the ML and picking
the patches. When the patch will be handled by someone is a different
question. As it is a high-traffic subsystem with an even higher ML
traffic, each sub-maintainer have its own policy about when they
review patches (usually one or twice per week - as most maintainers
are also active developers, and don't want to mix their development
time with reviewing time).

I'm not quite sure about what you expect with this specific part of
the profile.

I mean: why a submitter should care about office hours?

Also, people may be OOT during some period of time, or working
remotely from some other office.

Except if the idea would be to point to some site that would
dynamically track each maintainer's weekly maintainership
window (with would be a real pain to keep updated), I guess this
is useless.

Thanks,
Mauro
Dan Williams Nov. 18, 2018, 5:31 p.m. UTC | #21
On Sun, Nov 18, 2018 at 4:58 AM Mauro Carvalho Chehab
<mchehab@kernel.org> wrote:
>
> Em Fri, 16 Nov 2018 10:57:14 -0800
> Dan Williams <dan.j.williams@intel.com> escreveu:
> > On Fri, Nov 16, 2018 at 4:04 AM Mauro Carvalho Chehab
> > <mchehab@kernel.org> wrote:
[..]
> > Yes. Maybe a "Review Forum" section for subsystems that have
> > transitioned from email to a web-based tool? There's also the
> > exception of security disclosures, but the expectations for those
> > patches are already documented.
>
> Maybe. I would postpone adding a section like that until some
> subsystem maintainer that actually changed to Github/Gitlab
> would submit his subsystem profile.

Sure.

> > > > > +Last -rc for new feature submissions
[..]
> > > This is a general ruleset that describes the usual behavior, telling the
> > > developers the expected behavior. If the maintainers can do more on some
> > > particular development cycle, it should be fine.
> >
> > Yes, and perhaps I should clarify that this is the point at which a
> > maintainer will start to push back in the typical case, and indicate
> > to a contributor that they are standing in exceptional territory.
> > Similar to how later in the -rc series patches get increasing
> > scrutiny.
>
> Makes sense. There's one issue, though.
>
> I don't expect developers to read the profile template, as this is a
> material for the maintainer themselves. Developers should likely read
> just the specific subsystem profile for the patches that will be submitted.
>
> So, either each subsystem profile should have a reference to the
> profile template, or need to copy some "invariant" texts (with would be
> really painful to maintain).

Agree, a general link back to the handbook template for clarification
on any of the sections seems sufficient.

[..]
> > > > > +Trusted Reviewers
> > > > > +-----------------
> > > > > +While a maintainer / maintainer-team is expected to be reviewer of last
> > > > > +resort the review load is less onerous when distributed amongst
> > > > > +contributors and or a trusted set of individuals.  This section is
> > > > > +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> > > > > +reviewers that should always be copied on a patch submission, the
> > > > > +trusted reviewers here are individuals contributors can reach out to if
> > > > > +a few 'Resubmit Cadence' intervals have gone by without maintainer
> > > > > +action, or to otherwise consult for advice.
> > > >
> > > > This seems redundant with the MAINTAINERS reviewers list.  It seems like
> > > > the role specified in this section is more of an ombudsman or developer
> > > > advocate who can assist with the review and/or accept flow if the
> > > > maintainer is being slow to respond.
> > >
> > > Well, on subsystems that have sub-maintainers, there's no way to point to
> > > it at MAINTAINERS file.
> > >
> > > Also, not sure about others, but I usually avoid touching at existing
> > > MAINTAINERS file entries. This is a file that everyone touches, so it
> > > has higher chances of conflicts.
> > >
> > > Also, at least on media, we have 5 different API sets (digital TV, V4L2, CEC,
> > > media controller, remote controller). Yet, all drivers are stored at the
> > > same place (as a single driver may use multiple APIs).
> > >
> > > The reviewers for each API set are different. There isn't a good way
> > > to explain that inside a MAINTANERS file.
> >
> > Would it be worthwhile to have separate Subsystem Profiles for those
> > API reviewers? If they end up merging patches and sending them
> > upstream might we need a hierarchy of profiles for each hop along the
> > upstream merge path?
>
> I guess having hierarchical profiles will make it very confusing.
> The point is: inside a subsystem, the same ruleset usually applies to
> everything.

Ok.

> In the case of media, it is not uncommon to have patches that require
> multiple APIs. Consider, for example, a SoC used on a TV box. The driver
> itself should be placed at drivers/media/platform/, but it will end by
> being a bunch of sub-drivers that together will add support for V4L,
> Digital TV, remote controller, CEC and codecs, and need to be controlled
> via the media controller API. It may even have camera sensors.
>
> On other words, all media APIs will be used (after having it fully
> sent upstream).
>
> In practice, drivers for complex hardware like that is submitted in
> parts. For example, one SoC vendor started sending us the remote
> controller driver (as it would be the simplest one).
>
> The only part of the policy that changes, depending of what API
> is involved, is the one that will do the review.
>
> As the driver itself will be at the same place, no matter what APIs
> are used, get_maintainers.pl is not capable of identifying who are
> the reviewers based "F:" tags[1].
>
> [1] It could be possible to teach get_maintainers to better hint it,
> by making it look who are the reviewers for the headers that are
> included.
>
> >
> > > > > +Time Zone / Office Hours
> > > > > +------------------------
> > > > > +Let contributors know the time of day when one or more maintainers are
> > > > > +usually actively monitoring the mailing list.
> > > >
> > > > I would strike "actively monitoring the mailing list".  To me, it should
> > > > be what are the hours of the day that the maintainer might happen to poll
> > > > (or might receive an interrupt) from the appropriate communications
> > > > channels (could be IRC, could be email, etc).
> >
> > Yes, makes sense.
> >
> > > > For my area, I would want to say something like: I tend to be active
> > > > between 17:00 UTC (18:00 UTC when daylight savings) and 25:00 (26:00),
> > > > but often will check for urgent or brief items up until 07:00 (08:00).
> > > > I interact with email via a poll model.  I interact with IRC via a
> > > > pull model and often overlook IRC activity for multiple days).
> > >
> > > Frankly, for media, I don't think that working hours makes sense. Media
> > > (sub-)maintainers are spread around the globe, on different time zones
> > > (in US, Brazil and Europe). We also have several active developers in
> > > Japan, so we may end by having some day reviewers/sub-maintainers from
> > > there.
> >
> > For that case just say:
> >
> > "the sun never sets on the media subsystem" ;-)
>
> :-)
>
> >
> > > At max, we can say that we won't warrant to patches on weekends or holidays.
> >
> > Yeah, maybe:
> >
> > "outside of weekends or holidays there's usually a maintainer or
> > reviewer monitoring the mailing list"
>
> Well, 24/7, there is always patchwork monitoring the ML and picking
> the patches. When the patch will be handled by someone is a different
> question. As it is a high-traffic subsystem with an even higher ML
> traffic, each sub-maintainer have its own policy about when they
> review patches (usually one or twice per week - as most maintainers
> are also active developers, and don't want to mix their development
> time with reviewing time).
>
> I'm not quite sure about what you expect with this specific part of
> the profile.
>
> I mean: why a submitter should care about office hours?
>
> Also, people may be OOT during some period of time, or working
> remotely from some other office.
>
> Except if the idea would be to point to some site that would
> dynamically track each maintainer's weekly maintainership
> window (with would be a real pain to keep updated), I guess this
> is useless.

True, will remove. What's the point of stating daily active hours when
we already have "Resubmit Cadence" (I think I'll rename it "Follow
Cadence") measured in multiple days / weeks.
Dan Williams Nov. 18, 2018, 5:31 p.m. UTC | #22
On Sun, Nov 18, 2018 at 9:31 AM Dan Williams <dan.j.williams@intel.com> wrote:
>
>
> On Sun, Nov 18, 2018 at 4:58 AM Mauro Carvalho Chehab <mchehab@kernel.org> wrote:
> >
> > Em Fri, 16 Nov 2018 10:57:14 -0800
> > Dan Williams <dan.j.williams@intel.com> escreveu:
> > > On Fri, Nov 16, 2018 at 4:04 AM Mauro Carvalho Chehab
> > > <mchehab@kernel.org> wrote:
> [..]
> > > Yes. Maybe a "Review Forum" section for subsystems that have
> > > transitioned from email to a web-based tool? There's also the
> > > exception of security disclosures, but the expectations for those
> > > patches are already documented.
> >
> > Maybe. I would postpone adding a section like that until some
> > subsystem maintainer that actually changed to Github/Gitlab
> > would submit his subsystem profile.
>
> Sure.
>
> > > > > > +Last -rc for new feature submissions
> [..]
> > > > This is a general ruleset that describes the usual behavior, telling the
> > > > developers the expected behavior. If the maintainers can do more on some
> > > > particular development cycle, it should be fine.
> > >
> > > Yes, and perhaps I should clarify that this is the point at which a
> > > maintainer will start to push back in the typical case, and indicate
> > > to a contributor that they are standing in exceptional territory.
> > > Similar to how later in the -rc series patches get increasing
> > > scrutiny.
> >
> > Makes sense. There's one issue, though.
> >
> > I don't expect developers to read the profile template, as this is a
> > material for the maintainer themselves. Developers should likely read
> > just the specific subsystem profile for the patches that will be submitted.
> >
> > So, either each subsystem profile should have a reference to the
> > profile template, or need to copy some "invariant" texts (with would be
> > really painful to maintain).
>
> Agree, a general link back to the handbook template for clarification on any of the sections seems sufficient.
>
> [..]
> > > > > > +Trusted Reviewers
> > > > > > +-----------------
> > > > > > +While a maintainer / maintainer-team is expected to be reviewer of last
> > > > > > +resort the review load is less onerous when distributed amongst
> > > > > > +contributors and or a trusted set of individuals.  This section is
> > > > > > +distinct from the R: tag (Designated Reviewer). Whereas R: identifies
> > > > > > +reviewers that should always be copied on a patch submission, the
> > > > > > +trusted reviewers here are individuals contributors can reach out to if
> > > > > > +a few 'Resubmit Cadence' intervals have gone by without maintainer
> > > > > > +action, or to otherwise consult for advice.
> > > > >
> > > > > This seems redundant with the MAINTAINERS reviewers list.  It seems like
> > > > > the role specified in this section is more of an ombudsman or developer
> > > > > advocate who can assist with the review and/or accept flow if the
> > > > > maintainer is being slow to respond.
> > > >
> > > > Well, on subsystems that have sub-maintainers, there's no way to point to
> > > > it at MAINTAINERS file.
> > > >
> > > > Also, not sure about others, but I usually avoid touching at existing
> > > > MAINTAINERS file entries. This is a file that everyone touches, so it
> > > > has higher chances of conflicts.
> > > >
> > > > Also, at least on media, we have 5 different API sets (digital TV, V4L2, CEC,
> > > > media controller, remote controller). Yet, all drivers are stored at the
> > > > same place (as a single driver may use multiple APIs).
> > > >
> > > > The reviewers for each API set are different. There isn't a good way
> > > > to explain that inside a MAINTANERS file.
> > >
> > > Would it be worthwhile to have separate Subsystem Profiles for those
> > > API reviewers? If they end up merging patches and sending them
> > > upstream might we need a hierarchy of profiles for each hop along the
> > > upstream merge path?
> >
> > I guess having hierarchical profiles will make it very confusing.
> > The point is: inside a subsystem, the same ruleset usually applies to
> > everything.
>
> Ok.
>
> > In the case of media, it is not uncommon to have patches that require
> > multiple APIs. Consider, for example, a SoC used on a TV box. The driver
> > itself should be placed at drivers/media/platform/, but it will end by
> > being a bunch of sub-drivers that together will add support for V4L,
> > Digital TV, remote controller, CEC and codecs, and need to be controlled
> > via the media controller API. It may even have camera sensors.
> >
> > On other words, all media APIs will be used (after having it fully
> > sent upstream).
> >
> > In practice, drivers for complex hardware like that is submitted in
> > parts. For example, one SoC vendor started sending us the remote
> > controller driver (as it would be the simplest one).
> >
> > The only part of the policy that changes, depending of what API
> > is involved, is the one that will do the review.
> >
> > As the driver itself will be at the same place, no matter what APIs
> > are used, get_maintainers.pl is not capable of identifying who are
> > the reviewers based "F:" tags[1].
> >
> > [1] It could be possible to teach get_maintainers to better hint it,
> > by making it look who are the reviewers for the headers that are
> > included.
> >
> > >
> > > > > > +Time Zone / Office Hours
> > > > > > +------------------------
> > > > > > +Let contributors know the time of day when one or more maintainers are
> > > > > > +usually actively monitoring the mailing list.
> > > > >
> > > > > I would strike "actively monitoring the mailing list".  To me, it should
> > > > > be what are the hours of the day that the maintainer might happen to poll
> > > > > (or might receive an interrupt) from the appropriate communications
> > > > > channels (could be IRC, could be email, etc).
> > >
> > > Yes, makes sense.
> > >
> > > > > For my area, I would want to say something like: I tend to be active
> > > > > between 17:00 UTC (18:00 UTC when daylight savings) and 25:00 (26:00),
> > > > > but often will check for urgent or brief items up until 07:00 (08:00).
> > > > > I interact with email via a poll model.  I interact with IRC via a
> > > > > pull model and often overlook IRC activity for multiple days).
> > > >
> > > > Frankly, for media, I don't think that working hours makes sense. Media
> > > > (sub-)maintainers are spread around the globe, on different time zones
> > > > (in US, Brazil and Europe). We also have several active developers in
> > > > Japan, so we may end by having some day reviewers/sub-maintainers from
> > > > there.
> > >
> > > For that case just say:
> > >
> > > "the sun never sets on the media subsystem" ;-)
> >
> > :-)
> >
> > >
> > > > At max, we can say that we won't warrant to patches on weekends or holidays.
> > >
> > > Yeah, maybe:
> > >
> > > "outside of weekends or holidays there's usually a maintainer or
> > > reviewer monitoring the mailing list"
> >
> > Well, 24/7, there is always patchwork monitoring the ML and picking
> > the patches. When the patch will be handled by someone is a different
> > question. As it is a high-traffic subsystem with an even higher ML
> > traffic, each sub-maintainer have its own policy about when they
> > review patches (usually one or twice per week - as most maintainers
> > are also active developers, and don't want to mix their development
> > time with reviewing time).
> >
> > I'm not quite sure about what you expect with this specific part of
> > the profile.
> >
> > I mean: why a submitter should care about office hours?
> >
> > Also, people may be OOT during some period of time, or working
> > remotely from some other office.
> >
> > Except if the idea would be to point to some site that would
> > dynamically track each maintainer's weekly maintainership
> > window (with would be a real pain to keep updated), I guess this
> > is useless.
>
> True, will remove. What's the point of stating daily active hours when we already have "Resubmit Cadence" (I think I'll rename it "Follow Cadence") measured in multiple days / weeks.
Dan Williams Nov. 18, 2018, 5:34 p.m. UTC | #23
On Sun, Nov 18, 2018 at 9:31 AM Dan Williams <dan.j.williams@intel.com> wrote:
> On Sun, Nov 18, 2018 at 9:31 AM Dan Williams <dan.j.williams@intel.com> wrote:
[..]
> > True, will remove. What's the point of stating daily active hours when we already have "Resubmit Cadence" (I think I'll rename it "Follow Cadence") measured in multiple days / weeks.

Apologies, fumble-fingered that last mail. I meant to say "Follow-up
Cadence", and that include a maintainer preference for private pings,
or full patch set to be sent again to the list.
Mauro Carvalho Chehab Nov. 18, 2018, 5:44 p.m. UTC | #24
Em Sun, 18 Nov 2018 09:34:01 -0800
Dan Williams <dan.j.williams@intel.com> escreveu:

> On Sun, Nov 18, 2018 at 9:31 AM Dan Williams <dan.j.williams@intel.com> wrote:
> > On Sun, Nov 18, 2018 at 9:31 AM Dan Williams <dan.j.williams@intel.com> wrote:  
> [..]
> > > True, will remove. What's the point of stating daily active hours when we already have "Resubmit Cadence" (I think I'll rename it "Follow Cadence") measured in multiple days / weeks.  
> 
> Apologies, fumble-fingered that last mail. I meant to say "Follow-up
> Cadence", and that include a maintainer preference for private pings,
> or full patch set to be sent again to the list.

Yeah, "Follow-up Cadence" sounds a way better.

Thanks,
Mauro
Jani Nikula Nov. 20, 2018, 7:28 a.m. UTC | #25
On Fri, 16 Nov 2018, Joe Perches <joe@perches.com> wrote:
> On Fri, 2018-11-16 at 14:44 +0200, Jani Nikula wrote:
>> I quickly cooked up this script to produce the top-5 commit prefixes for
>> the given files over the arbitrary last 200 commits. It'll give you a
>> pretty good idea if you're even close.
>> 
>> ---
>> #!/bin/sh
>> # usage: subject-prefix FILE [...]
>> # show top 5 subject prefixes for FILEs
>> 
>> git log --format=%s -n 200 -- "$@" |\
>> 	grep -v "^Merge " |\
>> 	sed 's/\(.*\):.*/\1/' |\
>> 	sort | uniq -c | sort -nr | sed 's/ *[0-9]\+ //' |\
>> 	head -n 5
>> ---
>> 
>> Someone who knows perl could turn that into a checkpatch check: See if
>> the patch subject prefix is one of the top-5 for all files changed by
>> the patch, and ask the user to double check if it isn't. Or some
>> heuristics thereof.
>
> This won't work when a patch contains multiple files
> from different paths, or even multiple files from a
> single driver.

*shrug*

You can give it multiple files as argument, and it'll give you an
approximation of what the prefix could be, whether you're way off or
not. Close enough at least for the single driver case. Obviously not
perfect, but hey, it took me all of five minutes to write that. ;)

BR,
Jani.
diff mbox series

Patch

diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst
index 2a14916930cb..1e6b1aaa6024 100644
--- a/Documentation/maintainer/index.rst
+++ b/Documentation/maintainer/index.rst
@@ -11,4 +11,5 @@  additions to this manual.
 
    configure-git
    pull-requests
+   subsystem-profile
 
diff --git a/Documentation/maintainer/subsystem-profile.rst b/Documentation/maintainer/subsystem-profile.rst
new file mode 100644
index 000000000000..a74b624e0972
--- /dev/null
+++ b/Documentation/maintainer/subsystem-profile.rst
@@ -0,0 +1,145 @@ 
+.. _subsystemprofile:
+
+Subsystem Profile
+=================
+
+The Subsystem Profile is a collection of policy positions that a
+maintainer or maintainer team establishes for the their subsystem. While
+there is a wide range of technical nuance on maintaining disparate
+sections of the kernel, the Subsystem Profile documents a known set of
+major process policies that vary between subsystems.  What follows is a
+list of policy questions a maintainer can answer and include a document
+in the kernel, or on an external website. It advertises to other
+maintainers and contributors the local policy of the subsystem. Some
+sections are optional like "Overview", "Off-list review", and "TODO".
+The others are recommended for all subsystems to address, but no section
+is mandatory. In addition there are no wrong answers, just document how
+a subsystem typically operates. Note that the profile follows the
+subsystem not the maintainer, i.e. there is no expectation that a
+maintainer of multiple subsystems deploys the same policy across those
+subsystems.
+
+
+Overview
+--------
+In this optional section of the profile provide a free form overview of
+the subsystem written as a hand-off document. In other words write a
+note to someone that would receive the “keys to the castle” in the event
+of extended or unexpected absence. “So, you have recently become the
+maintainer of the XYZ subsystem, condolences, it is a thankless job,
+here is the lay of the land.” Details to consider are the extended
+details that are not included in MAINTAINERS, and not addressed by the
+other profile questions below. For example details like, who has access
+to the git tree, branches that are pulled into -next, relevant
+specifications, issue trackers, and sensitive code areas. If available
+the Overview should link to other subsystem documentation that may
+clarify, re-iterate, emphasize / de-emphasize portions of the global
+process documentation for contributors (CodingStyle, SubmittingPatches,
+etc...).
+
+
+Core
+----
+A list of F: tags (as described by MAINTAINERS) listing what the
+maintainer considers to be core files. The review and lead time
+constraints for 'core' code may be stricter given the increased
+sensitivity and risk of change.
+
+
+Patches or Pull requests
+------------------------
+Some subsystems allow contributors to send pull requests, most require
+mailed patches. State “Patches only”, or “Pull requests accepted”.
+
+
+Last -rc for new feature submissions
+------------------------------------
+New feature submissions targeting the next merge window should have
+their first posting for consideration before this point. Patches that
+are submitted after this point should be clear that they are targeting
+the NEXT+1 merge window, or should come with sufficient justification
+why they should be considered on an expedited schedule. A general
+guideline is to set expectation with contributors that new feature
+submissions should appear before -rc5. The answer may be different for
+'Core:' files, include a second entry prefixed with 'Core:' if so.
+
+
+Last -rc to merge features
+--------------------------
+Indicate to contributors the point at which an as yet un-applied patch
+set will need to wait for the NEXT+1 merge window. Of course there is no
+obligation to ever except any given patchset, but if the review has not
+concluded by this point the expectation the contributor should wait and
+resubmit for the following merge window. The answer may be different for
+'Core:' files, include a second entry prefixed with 'Core:' if so.
+
+
+Non-author Ack / Review Tags Required
+-------------------------------------
+Let contributors and other maintainers know whether they can expect to
+see the maintainer self-commit patches without 3rd-party review. Some
+subsystem developer communities are so small as to make this requirement
+impractical. Others may have been bootstrapped by a submission of
+self-reviewed code at the outset, but have since moved to a
+non-author review-required stance. This section sets expectations on the
+code-review economics in the subsystem. For example, can a contributor
+trade review of a maintainer's, or other contributor's patches in
+exchange for consideration of their own.
+
+
+Test Suite
+----------
+Indicate the test suite all patches are expected to pass before being
+submitted for inclusion consideration.
+
+
+Resubmit Cadence
+----------------
+Define a rate at which a contributor should wait to resubmit a patchset
+that has not yet received comments. A general guideline is to try to
+meet a deadline of 1 - 2 weeks to acknowledge starting consideration for
+a patch set.
+
+
+Trusted Reviewers
+-----------------
+While a maintainer / maintainer-team is expected to be reviewer of last
+resort the review load is less onerous when distributed amongst
+contributors and or a trusted set of individuals.  This section is
+distinct from the R: tag (Designated Reviewer). Whereas R: identifies
+reviewers that should always be copied on a patch submission, the
+trusted reviewers here are individuals contributors can reach out to if
+a few 'Resubmit Cadence' intervals have gone by without maintainer
+action, or to otherwise consult for advice.
+
+
+Time Zone / Office Hours
+------------------------
+Let contributors know the time of day when one or more maintainers are
+usually actively monitoring the mailing list.
+
+
+Checkpatch / Style Cleanups
+---------------------------
+For subsystems with long standing code bases it is reasonable to decline
+to accept pure coding-style fixup patches. This is where you can let
+contributors know “Standalone style-cleanups are welcome”,
+“Style-cleanups to existing code only welcome with other feature
+changes”, or “Standalone style-cleanups to existing code are not
+welcome”.
+
+
+Off-list review
+---------------
+A maintainer may optionally require that contributors seek prior review
+of patches before initial submission for upstream. For example,
+“Developers from organization X, please seek internal review before
+requesting upstream review”. This policy identifies occasions where a
+maintainer wants to reflect some of the review load back to an
+organization.
+
+
+TODO
+----
+In this optional section include a list of work items that might be
+suitable for onboarding a new developer to the subsystem.
diff --git a/MAINTAINERS b/MAINTAINERS
index 83b7b3943a12..bb4a83a7684d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -99,6 +99,10 @@  Descriptions of section entries:
 	   Obsolete:	Old code. Something tagged obsolete generally means
 			it has been replaced by a better system and you
 			should be using that.
+	P: Subsystem Profile document for the maintainer entry.  This
+	   is either an in-tree file or a URI to a document.  The
+	   contents of a Subsystem Profile are described in
+	   Documentation/maintainer/subsystem-profile.rst.
 	F: Files and directories with wildcard patterns.
 	   A trailing slash includes all files and subdirectory files.
 	   F:	drivers/net/	all files in and below drivers/net