Message-ID: <Z03Alg5lNTTDiFcF@valkosipuli.retiisi.eu>
Date: Mon, 2 Dec 2024 14:13:42 +0000
From: Sakari Ailus <sakari.ailus@....fi>
To: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc: linux-media@...r.kernel.org, Jonathan Corbet <corbet@....net>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	workflows@...r.kernel.org, Hans Verkuil <hverkuil@...ll.nl>,
	Ricardo Ribalda <ribalda@...omium.org>
Subject: Re: [PATCH v3 2/3] docs: media: document media multi-committers
 rules and process

Hi Mauro,

On Mon, Dec 02, 2024 at 10:26:20AM +0100, Mauro Carvalho Chehab wrote:
> As the media subsystem will experiment with a multi-committers model,
> update the Maintainer's entry profile to the new rules, and add a file
> documenting the process to become a committer and to maintain such
> rights.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> Signed-off-by: Hans Verkuil <hverkuil@...ll.nl>
> Reviewed-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
>  Documentation/driver-api/media/index.rst      |   1 +
>  .../media/maintainer-entry-profile.rst        |   8 +
>  .../driver-api/media/media-committer.rst      | 278 ++++++++++++++++++
>  .../process/maintainer-pgp-guide.rst          |   2 +
>  4 files changed, 289 insertions(+)
>  create mode 100644 Documentation/driver-api/media/media-committer.rst
> 
> diff --git a/Documentation/driver-api/media/index.rst b/Documentation/driver-api/media/index.rst
> index d5593182a3f9..d0c725fcbc67 100644
> --- a/Documentation/driver-api/media/index.rst
> +++ b/Documentation/driver-api/media/index.rst
> @@ -26,6 +26,7 @@ Documentation/userspace-api/media/index.rst
>      :numbered:
>  
>      maintainer-entry-profile
> +    media-committer
>  
>      v4l2-core
>      dtv-core
> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> index dc764163cf1c..705209eacf58 100644
> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> @@ -65,6 +65,9 @@ as described at Documentation/process/index.rst and to the Kernel
>  development rules inside the Kernel documentation, including its code of
>  conduct.
>  
> +More details about media commiters' roles and responsibilities can be
> +found here: Documentation/driver-api/media/media-committer.rst.
> +
>  Media development tree
>  ----------------------
>  
> @@ -200,6 +203,11 @@ shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
>  
>  With the pull request workflow, pull requests shall use a PGP-signed tag.
>  
> +With the committers' workflow, this is ensured at the time merge request
> +rights will be granted to the gitlab instance used by media-committers.git
> +tree, after receiving the e-mail documented at
> +:ref:`media-committer-agreement`.
> +
>  For more details about PGP sign, please read
>  Documentation/process/maintainer-pgp-guide.rst.
>  
> diff --git a/Documentation/driver-api/media/media-committer.rst b/Documentation/driver-api/media/media-committer.rst
> new file mode 100644
> index 000000000000..3c2f8f413307
> --- /dev/null
> +++ b/Documentation/driver-api/media/media-committer.rst
> @@ -0,0 +1,278 @@
> +Media committers
> +================
> +
> +What is a media committer?

"What" refers to a "thing" but developers generally are humans.

> +--------------------------
> +
> +A media committer is a developer who can push patches from other developers

s/can/has been granted commit access to/

?

> +and their own patches to the
> +`media-committers <https://gitlab.freedesktop.org/linux-media/media-committers>`_
> +tree.
> +
> +It is a media committer's duty to ensure that their entries in the MAINTAINERS
> +file are kept up-to-date, and that submitted patches for files for which
> +they are listed as maintainers are timely reviewed on the mailing list,
> +ideally not waiting in patchwork as ``New`` for more than one Kernel merge
> +cycle, and, if accepted, applying them at the media committer's tree.
> +
> +These commit rights are granted with some expectation of responsibility:

s/some //

> +committers are people who care about the Linux Kernel as a whole and
> +about the Linux media subsystem and want to help its development. It

s/help/advance/

?

> +is also based on a trust relationship between the rest of the committers,

s/also//
s/between the rest of/among/

I wonder if we should add here some expectation on being reachable on
#linux-media.

> +maintainers and the Linux Media community[1].
> +
> +As such, a media committer is not just someone who is capable of creating
> +code, but someone who has demonstrated their ability to collaborate
> +with the team, get the most knowledgeable people to review code,
> +contribute high-quality code, and follow through to fix issues (in code
> +or tests).
> +
> +.. Note::
> +
> +   1. If a patch introduces a regression, then it is the media committer's
> +      responsibility to correct that as soon as possible. Typically the
> +      patch is either reverted, or an additional patch is committed that
> +      fixes the regression;

s/that fixes/to fix/

> +   2. if patches are fixing bugs against already released Kernels, including
> +      the reverts above mentioned, the media committer shall add the needed
> +      tags. Please see :ref:`Media development workflow` for more details.

Does this reference work?

> +
> +[1] The Linux Media community, also called LinuxTV community, has its primary
> +    site at https://linuxtv.org.
> +
> +Becoming a media committer
> +--------------------------
> +
> +The most important aspect of volunteering to be a committer is that you have
> +demonstrated the ability to give good code reviews. So we are looking for

I wonder if we should add some kind of an expectation of demonstrating
common sense? :-)

> +whether or not we think you will be good at doing that.
> +
> +As such, potential committers must earn enough credibility and trust from the
> +LinuxTV community. To do that, developers shall be familiar with the open
> +source model and have been active in the Linux Kernel community for some time,
> +and, in particular, in the media subsystem.
> +
> +So, in addition to actually making the code changes, you are basically
> +demonstrating your:
> +
> +- commitment to the project;
> +- ability to collaborate with the team and communicate well;
> +- understand of how upstream and the LinuxTV community work
> +  (policies, processes for testing, code review, ...)
> +- reasonable knowledge about:
> +
> +  - the Kernel development process:
> +    Documentation/process/index.rst

:ref:`the Kernel development process <process_index>`

> +
> +  - the Media development profile:
> +    Documentation/driver-api/media/maintainer-entry-profile.rst

Could you add a label to the title and refer to it directly?

> +
> +- understanding of the projects' code base and coding style;
> +- ability to provide feedback to the patch authors;
> +- ability to judge when a patch might be ready for review and to submit;
> +- ability to write good code (last but certainly not least).
> +
> +Developers that intend to become committers are encouraged to participate

s/intend/yearn/

> +at the yearly Linux Media Summit, typically co-located with another Linux
> +conference.

s/another Linux/a Linux related/

> +
> +If you are doing such tasks and have become a valued developer, an
> +existing committer can nominate you to the media subsystem maintainers.
> +
> +The ultimate responsibility for accepting a nominated committer is up to
> +the subsystem's maintainers. The committers must earn a trust relationship
> +with all subsystem maintainers, as, by granting you commit rights, they will
> +be delegating part of their maintenance tasks.

s/delegating\K/ a/

> +
> +Due to that, to become a committer or a core committer, a consensus between
> +all subsystem maintainers is required, as they all need to trust a developer
> +well enough to be delegated the responsibility to maintain part of the code
> +and to properly review patches from third parties, in a timely manner and
> +keeping the status of the reviewed code at https://patchwork.linuxtv.org
> +updated.
> +
> +.. Note::
> +
> +   In order to preserve/protect the developers that could have their commit
> +   rights granted, denied or removed as well as the subsystem maintainers who
> +   have the task to accept or deny commit rights, all communication related to
> +   nominating a committer, preserving commit rights or leaving such function
> +   should happen in private as much as possible.
> +
> +.. _media-committer-agreement:
> +
> +Media committer's agreement
> +---------------------------
> +
> +Once a nominated committer is accepted by all subsystem maintainers,
> +they will ask if the developer is interested in the nomination and discuss
> +what area(s) of the media subsystem the committer will be responsible for.
> +
> +Once the developer accepts being a committer, the new committer shall
> +explicitly accept the Kernel development policies described under its
> +Documentation/, and, in particular to the rules on this document, by writing
> +an e-mail to media-committers@...uxtv.org, with a declaration of intent
> +following the model below::
> +
> +   I, John Doe, would like to change my status to: Committer
> +
> +   I intend to actively develop the XYZ driver, send fixes to drivers
> +   that I can test, optionally reviewing patches and merging trivial
> +   fixes in other areas of the subsystem, ...
> +
> +   For the purpose of committing patches to the media-committer's tree,
> +   I'll be using my user https://gitlab.freedesktop.org/users/<username>.
> +
> +Followed by a formal declaration of agreement with the Kernel development
> +rules::
> +
> +   I hereby declare that I agree with the Kernel development rules described at:
> +
> +   https://www.kernel.org/doc/html/latest/driver-api/media/media-committer.rst
> +
> +   and to the Linux Kernel development process rules.
> +
> +   I agree to the Code of Conduct as documented in:
> +   https://www.kernel.org/doc/html/latest/process/code-of-conduct.rst
> +
> +   I am aware that I can, at any point of time, retire. In that case, I will
> +   send an e-mail to notify the subsystem maintainers for them to revoke my
> +   commit rights.
> +
> +   I am aware that the Kernel development rules change over time.
> +   By doing a new push to media-commiter tree, I understand that I agree
> +   with the rules in effect at the time of the commit.
> +
> +Such e-mail shall be signed with a PGP key cross signed by other Kernel and
> +media developers. As described at :ref:`media-developers-gpg`, the PGP
> +signature, together with the gitlab user security are fundamental components
> +that ensure the authentity of the merge requests that will happen at the
> +media-committer.git tree.
> +
> +In case the kernel development process changes, by merging new commits
> +in the
> +`media-committer tree <https://gitlab.freedesktop.org/linux-media/media-committers>`_,
> +the media committer implicitly declares their agreement with the latest
> +version of the documented process including the contents of this file.
> +
> +.. note::
> +
> +   1. Changes to the kernel media development process should be announced in

s/should/shall/ ?

> +      the media-committers mailinglist with a reasonable review period. All
> +      committers are automatically subscribed to that mailinglist;
> +   2. Due to the distributed nature of the Kernel development, it is
> +      possible that kernel development process changes may end being
> +      reviewed/merged at the linux-docs mailing list, specially for the
> +      contents under Documentation/process and for trivial typo fixes.
> +
> +Core committers
> +---------------
> +
> +As described in Documentation/driver-api/media/maintainer-entry-profile.rst
> +a committer may be granted with additional rights to also be able to
> +change a core file and/or media subsystem's Kernel API. The extent of
> +the core committer's grants will be detailed by the subsystem maintainers
> +when they nominate a core committer.
> +
> +Existing committers may become core committers and vice versa. Such
> +decisions will be taken in consensus between the subsystem maintainers.
> +
> +Media committers rules
> +----------------------
> +
> +Media committers shall do their best efforts to avoid merged patches that
> +would break any existing drivers. If it breaks, fixup or revert patches
> +shall be merged as soon as possible, aiming to be merged at the same Kernel
> +cycle the bug is reported.
> +
> +Media committers shall behave accordingly to the rights granted by
> +the subsystem maintainers, specially with regards of the scope of changes
> +they may apply directly at the media-committers tree. Such scope can
> +change over time on a mutual agreement between media committers and
> +maintainers.
> +
> +As described at :ref:`Media development workflow`, there are workflows.
> +For the committers' workflow, the following rules apply:
> +
> +- Each merged patch shall pass CI tests;
> +
> +- Media committers shall request reviews from other committers and
> +  developers where applicable, i.e. because those developers have more
> +  knowledge about some areas that are changed by a patch;
> +
> +- There shall be no open issues or unresolved or conflicting feedback
> +  from anyone. Clear them up first. Defer to subsystem maintainers as needed.
> +
> +Patches that do not fall under the committer's workflow criteria will follow
> +the pull request workflow as described at :ref:`Media development workflow`.
> +
> +Only a subsystem maintainer can override such rules.
> +
> +All media committers shall ensure that patchwork will reflect the current
> +status, e.g. patches shall be delegated to the media committer who is
> +handling them and the patch status shall be updated according to these rules:
> +
> +- ``Under review``: Used if the patch requires a second opinion
> +  or when it is part of a pull request;
> +- ``Accepted``: Once a patch is merged in the multi-committer tree.
> +- ``Superseded``: There is a newer version of the patch posted to the
> +  mailing list.
> +- ``Duplicated``: There was another patch doing the same thing from someone
> +  else that was accepted.
> +- ``Not Applicable``: Use for patch series that are not merged at media.git
> +  tree (e.g. drm, dmabuf, upstream merge, etc.) but were cross-posted to the
> +  linux-media mailing list.
> +
> +If the committer decides not to merge it, then reply by email to patch
> +authors, explaining why it is not merged, and patchwork shall be updated
> +accordingly with either:
> +
> +- ``Changes Requested``: if a new revision was requested;
> +- ``Rejected``: if the proposed change won't be merged upstream.
> +
> +If a media committer decides to retire, it is the committer's duty to
> +notify the subsystem maintainers about that decision.
> +
> +.. Note::
> +
> +   Patchwork supports a couple of clients to help semi-automating
> +   status updates via its REST interface:
> +
> +   https://patchwork.readthedocs.io/en/latest/usage/clients/
> +
> +Maintaining media committer status
> +----------------------------------
> +
> +A community of committers working together to move the Linux Kernel
> +forward is essential to creating successful projects that are rewarding
> +to work on. If there are problems or disagreements within the community,
> +they can usually be solved through healthy discussion and debate.
> +
> +In the unhappy event that a media committer continues to disregard good
> +citizenship (or actively disrupts the project), we may need to revoke
> +that person's status. In such cases, if someone suggests the revocation
> +with a good reason, then after discussing this among the media committers,
> +the final decision is taken by the subsystem maintainers. As the decision
> +to become a media committer comes from a consensus between subsystem
> +maintainers, a single subsystem maintainer not trusting the media committer
> +anymore is enough to revoke committer's grants.
> +
> +If a committer is inactive for more than a couple of Kernel cycles,
> +maintainers will try to reach you via e-mail. If not possible, they may
> +revoke your committer grants and update MAINTAINERS file entries
> +accordingly. If you wish to resume contributing later on, then contact
> +the subsystem maintainers to ask if your rights can be restored.
> +
> +A previous committer that had their commit rights revoked can keep
> +contributing to the subsystem via the pull request workflow as documented
> +at the :ref:`Media development workflow`.
> +
> +References
> +----------
> +
> +Much of this was inspired by/copied from the committer policies of:
> +
> +- `Chromium <https://chromium.googlesource.com/chromium/src/+/main/docs/contributing.md>`_;
> +- `WebKit <https://webkit.org/commit-and-review-policy/>`_;
> +- `Mozilla <https://www.mozilla.org/hacking/committer/>`_.
> +
> diff --git a/Documentation/process/maintainer-pgp-guide.rst b/Documentation/process/maintainer-pgp-guide.rst
> index f5277993b195..795ef8d89271 100644
> --- a/Documentation/process/maintainer-pgp-guide.rst
> +++ b/Documentation/process/maintainer-pgp-guide.rst
> @@ -903,6 +903,8 @@ the new default in GnuPG v2). To set it, add (or modify) the
>  
>      trust-model tofu+pgp
>  
> +.. _kernel_org_trust_repository:
> +
>  Using the kernel.org web of trust repository
>  --------------------------------------------
>  

-- 
Kind regards,

Sakari Ailus

Powered by blists - more mailing lists