public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Document the new media-committer's model
@ 2024-12-02  9:26 Mauro Carvalho Chehab
  2024-12-02  9:26 ` [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers Mauro Carvalho Chehab
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-02  9:26 UTC (permalink / raw)
  To: linux-media
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Mauro Carvalho Chehab,
	linux-doc, linux-kernel, workflows

The media subsystem used to have a multi-commiter's model in the
past, but things didn't go well on that time, and we had to move to
a centralized model.

As the community has evolved, and as there are now new policies in
place like CoC, let's experiment with a multi-committers again.

The model we're using was inspired by the DRM multi-committers
model. Yet, media subsystem is different on several aspects, so the
model is not exactly the same.

The implementation will be in phases. For this phase, the goal is that 
all committers will be people listed at MAINTAINERS.

On this series,:

patch 1: updates the  media maintainer's entry profile and adds the
workflow that will be used with the new model. While here, it also
adds a missing "P:" tag at the MAINTAINERS file, pointing to it;

patch 2: adds a new document focused at the new maintainers
process. Its target is for developers that will be granted with
commit rights at the new media-maintainers.git tree. It also
contains a reference tag addition to kernel.org PGP chain
at process/maintainer-pgp-guide.rst.

patch 3: make documents cleared about maintainership duties.

---

v3:
- added patch 3;
- addressed nits pointed by Ricardo during his review;
- did minor editorial changes to improve Sphinx html output.

v2: I tried to address most of the suggestions where there was an agreement
from Laurent's review and further comments. As there were several changes,
on separate threads, I could have missed something.


Mauro Carvalho Chehab (3):
  docs: media: update maintainer-entry-profile for multi-committers
  docs: media: document media multi-committers rules and process
  docs: media: profile: make it clearer about maintainership duties

 Documentation/driver-api/media/index.rst      |   1 +
 .../media/maintainer-entry-profile.rst        | 219 +++++++++++---
 .../driver-api/media/media-committer.rst      | 278 ++++++++++++++++++
 .../process/maintainer-pgp-guide.rst          |   2 +
 MAINTAINERS                                   |   1 +
 5 files changed, 456 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/driver-api/media/media-committer.rst

-- 
2.47.1



^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02  9:26 [PATCH v3 0/3] Document the new media-committer's model Mauro Carvalho Chehab
@ 2024-12-02  9:26 ` Mauro Carvalho Chehab
  2024-12-02 12:40   ` Sakari Ailus
  2024-12-02 13:17   ` Hans Verkuil
  2024-12-02  9:26 ` [PATCH v3 2/3] docs: media: document media multi-committers rules and process Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-02  9:26 UTC (permalink / raw)
  To: linux-media
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
	Hans Verkuil, Ricardo Ribalda

As the media subsystem will experiment with a multi-committers model,
update the Maintainer's entry profile to the new rules.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
---
 .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
 MAINTAINERS                                   |   1 +
 2 files changed, 163 insertions(+), 46 deletions(-)

diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
index ffc712a5f632..dc764163cf1c 100644
--- a/Documentation/driver-api/media/maintainer-entry-profile.rst
+++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
@@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
 Both media userspace and Kernel APIs are documented and the documentation
 must be kept in sync with the API changes. It means that all patches that
 add new features to the subsystem must also bring changes to the
-corresponding API files.
+corresponding API documentation files.
 
-Due to the size and wide scope of the media subsystem, media's
-maintainership model is to have sub-maintainers that have a broad
-knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
-task to review the patches, providing feedback to users if the patches are
+Due to the size and wide scope of the media subsystem, the media's
+maintainership model is to have committers that have a broad knowledge of
+a specific aspect of the subsystem. It is the committers' task to
+review the patches, providing feedback to users if the patches are
 following the subsystem rules and are properly using the media kernel and
 userspace APIs.
 
-Patches for the media subsystem must be sent to the media mailing list
-at linux-media@vger.kernel.org as plain text only e-mail. Emails with
-HTML will be automatically rejected by the mail server. It could be wise
-to also copy the sub-maintainer(s).
+Media committers
+----------------
+
+In the media subsystem, there are experienced developers who can push
+patches directly to the development tree. These developers are called
+Media committers and are divided into the following categories:
+
+- Committers:
+    contributors for one or more drivers within the media subsystem.
+    They can push changes to the tree that do not affect the core or ABI.
+
+- Core committers:
+    responsible for part of the media core. They are typically
+    responsible for one or more drivers within the media subsystem, but, besides
+    that, they can also merge patches that change the code common to multiple
+    drivers, including the kernel internal API.
+
+- Subsystem maintainers:
+    responsible for the subsystem as a whole, with access to the
+    entire subsystem.
+
+    Only subsystem maintainers can push changes that affect the userspace
+    API/ABI.
+
+All media committers shall explicitly agree with the Kernel development process
+as described at Documentation/process/index.rst and to the Kernel
+development rules inside the Kernel documentation, including its code of
+conduct.
+
+Media development tree
+----------------------
+
+The main development tree used by the media subsystem is hosted at LinuxTV.org,
+where we also maintain news about the subsystem, wiki pages and a patchwork
+instance where we track patches though their lifetime.
+
+The main tree used by media developers is at:
+
+https://git.linuxtv.org/media.git/
+
+.. _Media development workflow:
+
+Media development workflow
+++++++++++++++++++++++++++
+
+All changes for the media subsystem must be sent first as e-mails to the
+media mailing list, following the process documented at
+Documentation/process/index.rst.
+
+It means that patches shall be submitted as plain text only via e-mail to
+linux-media@vger.kernel.org. While subscription is not mandatory, you
+can find details about how to subscribe to it and to see its archives at:
+
+  https://subspace.kernel.org/vger.kernel.org.html
+
+Emails with HTML will be automatically rejected by the mail server.
+
+It could be wise to also copy the media committer(s). You should use
+``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
+Please always copy driver's authors and maintainers.
+
+Such patches need to be based against a public branch or tag as follows:
+
+1. Patches for new features need to be based at the ``next`` branch of
+   media.git tree;
+
+2. Fixes against an already released kernel should preferably be against
+   the latest released Kernel. If they require a previously-applied
+   change at media.git tree, they need to be against its ``fixes`` branch.
+
+3. Fixes for issues not present at the latest released kernel shall
+   be either against a -rc kernel for an upcoming release or
+   against the ``fixes`` branch of the media.git tree.
+
+Patches with fixes shall have:
+
+- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
+- when applicable, a ``Cc: stable@vger.kernel.org``.
+
+Patches that were fixing bugs publicly reported by someone at the
+linux-media@vger.kernel.org mailing list shall have:
+
+- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
+
+Patches that change API shall update documentation accordingly at the
+same patch series.
+
+See Documentation/process/index.rst for more details about e-mail submission.
+
+Once a patch is submitted, it may follow either one of the following
+workflows:
+
+a. Pull request workflow: patches are handled by subsystem maintainers::
+
+     +------+   +---------+   +-------+   +-----------------------+   +---------+
+     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
+     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
+                +---------+   +-------+   +-----------------------+
+
+   For this workflow, pull requests can be generated by a committer,
+   a previous committer, subsystem maintainers or by a trusted long-time
+   contributor. If you are not in such group, please don't submit
+   pull requests, as they will not be processed.
+
+b. Committers' workflow: patches are handled by media committers::
+
+     +------+   +---------+   +--------------------+   +-----------+   +---------+
+     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
+     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
+                +---------+   +--------------------+   +-----------+
+
+On both workflows, all patches shall be properly reviewed at
+linux-media@vger.kernel.org before being merged at media-committers.git.
+
+When patches are picked by patchwork and when merged at media-committers,
+CI bots will check for errors and may provide e-mail feedback about
+patch problems. When this happens, the patch submitter must fix them, or
+explain why the errors are false positives.
+
+Patches will only be moved to the next stage in those two workflows if they
+don't fail on CI or if there are false-positives in the CI reports.
+
+Failures during e-mail submission
++++++++++++++++++++++++++++++++++
 
 Media's workflow is heavily based on Patchwork, meaning that, once a patch
 is submitted, the e-mail will first be accepted by the mailing list
@@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
 
    - https://patchwork.linuxtv.org/project/linux-media/list/
 
-If it doesn't automatically appear there after a few minutes, then
+If it doesn't automatically appear there after some time [2]_, then
 probably something went wrong on your submission. Please check if the
-email is in plain text\ [2]_ only and if your emailer is not mangling
+email is in plain text\ [3]_ only and if your emailer is not mangling
 whitespaces before complaining or submitting them again.
 
-You can check if the mailing list server accepted your patch, by looking at:
+To troubleshoot problems, you should first check if the mailing list
+server has accepted your patch, by looking at:
 
    - https://lore.kernel.org/linux-media/
 
-.. [2] If your email contains HTML, the mailing list server will simply
+If the patch is there and not at patchwork, it is likely that your e-mailer
+mangled the patch. Patchwork internally has a logic that checks if the
+received e-mail contain a valid patch. Any whitespace and new line
+breakages mangling the patch won't be recognized by patchwork, thus such
+patch will be rejected.
+
+.. [2] It usually takes a few minutes for the patch to arrive, but
+       the e-mail server may be busy, so it may take up to a few hours
+       for a patch to be picked by patchwork.
+
+.. [3] If your email contains HTML, the mailing list server will simply
        drop it, without any further notice.
 
+.. _media-developers-gpg:
 
-Media maintainers
-+++++++++++++++++
+Authentication for pull and merge requests
+++++++++++++++++++++++++++++++++++++++++++
 
-At the media subsystem, we have a group of senior developers that
-are responsible for doing the code reviews at the drivers (also known as
-sub-maintainers), and another senior developer responsible for the
-subsystem as a whole. For core changes, whenever possible, multiple
-media maintainers do the review.
+The authenticity of developers submitting pull requests and merge requests
+shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
 
-The media maintainers that work on specific areas of the subsystem are:
+With the pull request workflow, pull requests shall use a PGP-signed tag.
 
-- Remote Controllers (infrared):
-    Sean Young <sean@mess.org>
+For more details about PGP sign, please read
+Documentation/process/maintainer-pgp-guide.rst.
 
-- HDMI CEC:
-    Hans Verkuil <hverkuil@xs4all.nl>
+Subsystem maintainers
+---------------------
 
-- Media controller drivers:
-    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
-
-- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
-    Sakari Ailus <sakari.ailus@linux.intel.com>
-
-- V4L2 drivers and core V4L2 frameworks:
-    Hans Verkuil <hverkuil@xs4all.nl>
-
-The subsystem maintainer is:
-  Mauro Carvalho Chehab <mchehab@kernel.org>
-
-Media maintainers may delegate a patch to other media maintainers as needed.
-On such case, checkpatch's ``delegate`` field indicates who's currently
-responsible for reviewing a patch.
+The subsystem maintainers are:
+  - Mauro Carvalho Chehab <mchehab@kernel.org> and
+  - Hans Verkuil <hverkuil@xs4all.nl>
 
 Submit Checklist Addendum
 -------------------------
@@ -108,17 +225,14 @@ implementing the media APIs:
 ====================	=======================================================
 Type			Tool
 ====================	=======================================================
-V4L2 drivers\ [3]_	``v4l2-compliance``
+V4L2 drivers\ [4]_	``v4l2-compliance``
 V4L2 virtual drivers	``contrib/test/test-media``
 CEC drivers		``cec-compliance``
 ====================	=======================================================
 
-.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
+.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
        V4L2 drivers.
 
-Other compilance tools are under development to check other parts of the
-subsystem.
-
 Those tests need to pass before the patches go upstream.
 
 Also, please notice that we build the Kernel with::
@@ -134,6 +248,8 @@ Where the check script is::
 Be sure to not introduce new warnings on your patches without a
 very good reason.
 
+Please see `Media development workflow`_ for e-mail submission rules.
+
 Style Cleanup Patches
 +++++++++++++++++++++
 
@@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
 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 in a couple of weeks or to ask
-other developers to publicly add Reviewed-by and, more importantly,
+other developers to publicly add ``Reviewed-by:`` and, more importantly,
 ``Tested-by:`` tags.
 
 Please note that we expect a detailed description for ``Tested-by:``,
diff --git a/MAINTAINERS b/MAINTAINERS
index 1e930c7a58b1..c77f56a2e695 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
 M:	Mauro Carvalho Chehab <mchehab@kernel.org>
 L:	linux-media@vger.kernel.org
 S:	Maintained
+P:	Documentation/driver-api/media/maintainer-entry-profile.rst
 W:	https://linuxtv.org
 Q:	http://patchwork.kernel.org/project/linux-media/list/
 T:	git git://linuxtv.org/media.git
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 2/3] docs: media: document media multi-committers rules and process
  2024-12-02  9:26 [PATCH v3 0/3] Document the new media-committer's model Mauro Carvalho Chehab
  2024-12-02  9:26 ` [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers Mauro Carvalho Chehab
@ 2024-12-02  9:26 ` Mauro Carvalho Chehab
  2024-12-02 14:13   ` Sakari Ailus
  2024-12-02 14:37   ` Hans Verkuil
  2024-12-02  9:26 ` [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties Mauro Carvalho Chehab
  2024-12-02 15:03 ` [PATCH v3 0/3] Document the new media-committer's model Hans Verkuil
  3 siblings, 2 replies; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-02  9:26 UTC (permalink / raw)
  To: linux-media
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Mauro Carvalho Chehab,
	linux-doc, linux-kernel, workflows, Hans Verkuil, Ricardo Ribalda

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@kernel.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.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?
+--------------------------
+
+A media committer is a developer who can push patches from other developers
+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:
+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
+is also based on a trust relationship between the rest of the committers,
+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;
+   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.
+
+[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
+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
+
+  - the Media development profile:
+    Documentation/driver-api/media/maintainer-entry-profile.rst
+
+- 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
+at the yearly Linux Media Summit, typically co-located with another Linux
+conference.
+
+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.
+
+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@linuxtv.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
+      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
 --------------------------------------------
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties
  2024-12-02  9:26 [PATCH v3 0/3] Document the new media-committer's model Mauro Carvalho Chehab
  2024-12-02  9:26 ` [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers Mauro Carvalho Chehab
  2024-12-02  9:26 ` [PATCH v3 2/3] docs: media: document media multi-committers rules and process Mauro Carvalho Chehab
@ 2024-12-02  9:26 ` Mauro Carvalho Chehab
  2024-12-02 14:40   ` Hans Verkuil
  2024-12-02 14:48   ` Sakari Ailus
  2024-12-02 15:03 ` [PATCH v3 0/3] Document the new media-committer's model Hans Verkuil
  3 siblings, 2 replies; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-02  9:26 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel

During the review of the media committes profile, it was noticed
that the responsibility for timely review patches was not clear:
such review is expected that all developers listed at MAINTAINERS
with the "M:" tag (e.g. "maintainers" on its broad sense).

This is orthogonal of being a media committer or not. Such duty
is implied at:

	Documentation/admin-guide/reporting-issues.rst

and at the MAINTAINERS header, when it says that even when the
status is "odd fixes", the patches will flow in.

So, let make it explicit at the maintainer-entry-profile that
maintainers need to do timely reviews.

Also, while right now our focus is on granting committer rights to
maintainers, the media-committer model may evolve in the future to
accept other committers that don't have such duties.

So, make it clear at the media-committer.rst that the duties
related to reviewing patches from others are for the drivers
they are maintainers as well.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 Documentation/driver-api/media/maintainer-entry-profile.rst | 5 +++++
 Documentation/driver-api/media/media-committer.rst          | 6 +++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
index 705209eacf58..50568c744129 100644
--- a/Documentation/driver-api/media/maintainer-entry-profile.rst
+++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
@@ -153,6 +153,11 @@ b. Committers' workflow: patches are handled by media committers::
 On both workflows, all patches shall be properly reviewed at
 linux-media@vger.kernel.org before being merged at media-committers.git.
 
+Such patches will be timely-reviewed by developers listed as maintainers at
+the MAINTAINERS file. Such maintainers will follow one of the above
+workflows, e. g. they will either send a pull request or merge patches
+directly at the media-committers tree.
+
 When patches are picked by patchwork and when merged at media-committers,
 CI bots will check for errors and may provide e-mail feedback about
 patch problems. When this happens, the patch submitter must fix them, or
diff --git a/Documentation/driver-api/media/media-committer.rst b/Documentation/driver-api/media/media-committer.rst
index 3c2f8f413307..ec81f01db126 100644
--- a/Documentation/driver-api/media/media-committer.rst
+++ b/Documentation/driver-api/media/media-committer.rst
@@ -87,9 +87,9 @@ be delegating part of their maintenance tasks.
 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.
+and to properly review patches from third parties for the drivers they are
+maintainers in a timely manner and keeping the status of the reviewed code
+at https://patchwork.linuxtv.org updated.
 
 .. Note::
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02  9:26 ` [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers Mauro Carvalho Chehab
@ 2024-12-02 12:40   ` Sakari Ailus
  2024-12-02 13:23     ` Mauro Carvalho Chehab
  2024-12-02 13:17   ` Hans Verkuil
  1 sibling, 1 reply; 26+ messages in thread
From: Sakari Ailus @ 2024-12-02 12:40 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Hans Verkuil, Ricardo Ribalda

Hi Mauro,

Thanks for the set.

Looks good overall, please still see my comments below.

On Mon, Dec 02, 2024 at 10:26:19AM +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.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
>  MAINTAINERS                                   |   1 +
>  2 files changed, 163 insertions(+), 46 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> index ffc712a5f632..dc764163cf1c 100644
> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
>  Both media userspace and Kernel APIs are documented and the documentation
>  must be kept in sync with the API changes. It means that all patches that
>  add new features to the subsystem must also bring changes to the
> -corresponding API files.
> +corresponding API documentation files.
>  
> -Due to the size and wide scope of the media subsystem, media's
> -maintainership model is to have sub-maintainers that have a broad
> -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> -task to review the patches, providing feedback to users if the patches are
> +Due to the size and wide scope of the media subsystem, the media's
> +maintainership model is to have committers that have a broad knowledge of
> +a specific aspect of the subsystem. It is the committers' task to
> +review the patches, providing feedback to users if the patches are
>  following the subsystem rules and are properly using the media kernel and
>  userspace APIs.
>  
> -Patches for the media subsystem must be sent to the media mailing list
> -at linux-media@vger.kernel.org as plain text only e-mail. Emails with
> -HTML will be automatically rejected by the mail server. It could be wise
> -to also copy the sub-maintainer(s).
> +Media committers
> +----------------
> +
> +In the media subsystem, there are experienced developers who can push

The media subsystem is generally understood to comprise of what's under
drivers/media, this should be referring to the community instead.

> +patches directly to the development tree. These developers are called
> +Media committers and are divided into the following categories:
> +
> +- Committers:
> +    contributors for one or more drivers within the media subsystem.
> +    They can push changes to the tree that do not affect the core or ABI.
> +
> +- Core committers:
> +    responsible for part of the media core. They are typically
> +    responsible for one or more drivers within the media subsystem, but, besides
> +    that, they can also merge patches that change the code common to multiple
> +    drivers, including the kernel internal API.
> +
> +- Subsystem maintainers:

s/Subsystem/Media tree/

?
> +    responsible for the subsystem as a whole, with access to the
> +    entire subsystem.
> +
> +    Only subsystem maintainers can push changes that affect the userspace
> +    API/ABI.

This is ambiguous. I think it should intend to say API/ABI changes require
approval from Media tree maintainers.

What constitutes a UAPI change is a topic of discussion on its own. Does it
require adding a new IOCTL? Taking into use a reserved field? Changing
little-used driver behaviour slightly? Fixing a bug in a driver?

The first two obviously yes, but the latter two probably not.

Also:

s/Only subsystem maintainers can push/Media tree maintainers' ack is
required for/

?

> +
> +All media committers shall explicitly agree with the Kernel development process
> +as described at Documentation/process/index.rst and to the Kernel
> +development rules inside the Kernel documentation, including its code of
> +conduct.
> +
> +Media development tree
> +----------------------
> +
> +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> +where we also maintain news about the subsystem, wiki pages and a patchwork
> +instance where we track patches though their lifetime.
> +
> +The main tree used by media developers is at:
> +
> +https://git.linuxtv.org/media.git/
> +
> +.. _Media development workflow:
> +
> +Media development workflow
> +++++++++++++++++++++++++++
> +
> +All changes for the media subsystem must be sent first as e-mails to the
> +media mailing list, following the process documented at

s/^/linux-/

Also I'd refer to it as "LMML".

> +Documentation/process/index.rst.
> +
> +It means that patches shall be submitted as plain text only via e-mail to
> +linux-media@vger.kernel.org. While subscription is not mandatory, you
> +can find details about how to subscribe to it and to see its archives at:
> +
> +  https://subspace.kernel.org/vger.kernel.org.html
> +
> +Emails with HTML will be automatically rejected by the mail server.
> +
> +It could be wise to also copy the media committer(s). You should use
> +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> +Please always copy driver's authors and maintainers.
> +
> +Such patches need to be based against a public branch or tag as follows:
> +
> +1. Patches for new features need to be based at the ``next`` branch of
> +   media.git tree;
> +
> +2. Fixes against an already released kernel should preferably be against
> +   the latest released Kernel. If they require a previously-applied
> +   change at media.git tree, they need to be against its ``fixes`` branch.

This would be a change to the current process that I don't think has been
discussed. If I understand correctly, generally this applies to patches
that have been merged to the development branch (formerly media stage tree
master) with Cc: stable and Fixes: tags.

> +
> +3. Fixes for issues not present at the latest released kernel shall
> +   be either against a -rc kernel for an upcoming release or
> +   against the ``fixes`` branch of the media.git tree.
> +
> +Patches with fixes shall have:
> +
> +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> +- when applicable, a ``Cc: stable@vger.kernel.org``.
> +
> +Patches that were fixing bugs publicly reported by someone at the
> +linux-media@vger.kernel.org mailing list shall have:
> +
> +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> +
> +Patches that change API shall update documentation accordingly at the
> +same patch series.
> +
> +See Documentation/process/index.rst for more details about e-mail submission.
> +
> +Once a patch is submitted, it may follow either one of the following
> +workflows:
> +
> +a. Pull request workflow: patches are handled by subsystem maintainers::
> +
> +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|

s/e-mail/LMML/

(see earlier comment)? Same below.

> +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> +                +---------+   +-------+   +-----------------------+
> +
> +   For this workflow, pull requests can be generated by a committer,
> +   a previous committer, subsystem maintainers or by a trusted long-time

s/previous/former/

I'd also use plural in all cases here.

> +   contributor. If you are not in such group, please don't submit

> +   pull requests, as they will not be processed.
> +
> +b. Committers' workflow: patches are handled by media committers::
> +
> +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> +                +---------+   +--------------------+   +-----------+
> +
> +On both workflows, all patches shall be properly reviewed at
> +linux-media@vger.kernel.org before being merged at media-committers.git.
> +
> +When patches are picked by patchwork and when merged at media-committers,
> +CI bots will check for errors and may provide e-mail feedback about
> +patch problems. When this happens, the patch submitter must fix them, or
> +explain why the errors are false positives.
> +
> +Patches will only be moved to the next stage in those two workflows if they
> +don't fail on CI or if there are false-positives in the CI reports.

s/don't fail on/pass/

> +
> +Failures during e-mail submission
> ++++++++++++++++++++++++++++++++++
>  
>  Media's workflow is heavily based on Patchwork, meaning that, once a patch
>  is submitted, the e-mail will first be accepted by the mailing list
> @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
>  
>     - https://patchwork.linuxtv.org/project/linux-media/list/
>  
> -If it doesn't automatically appear there after a few minutes, then
> +If it doesn't automatically appear there after some time [2]_, then
>  probably something went wrong on your submission. Please check if the
> -email is in plain text\ [2]_ only and if your emailer is not mangling
> +email is in plain text\ [3]_ only and if your emailer is not mangling
>  whitespaces before complaining or submitting them again.
>  
> -You can check if the mailing list server accepted your patch, by looking at:
> +To troubleshoot problems, you should first check if the mailing list
> +server has accepted your patch, by looking at:
>  
>     - https://lore.kernel.org/linux-media/
>  
> -.. [2] If your email contains HTML, the mailing list server will simply
> +If the patch is there and not at patchwork, it is likely that your e-mailer
> +mangled the patch. Patchwork internally has a logic that checks if the
> +received e-mail contain a valid patch. Any whitespace and new line
> +breakages mangling the patch won't be recognized by patchwork, thus such
> +patch will be rejected.
> +
> +.. [2] It usually takes a few minutes for the patch to arrive, but
> +       the e-mail server may be busy, so it may take up to a few hours
> +       for a patch to be picked by patchwork.
> +
> +.. [3] If your email contains HTML, the mailing list server will simply
>         drop it, without any further notice.
>  
> +.. _media-developers-gpg:
>  
> -Media maintainers
> -+++++++++++++++++
> +Authentication for pull and merge requests
> +++++++++++++++++++++++++++++++++++++++++++
>  
> -At the media subsystem, we have a group of senior developers that
> -are responsible for doing the code reviews at the drivers (also known as
> -sub-maintainers), and another senior developer responsible for the
> -subsystem as a whole. For core changes, whenever possible, multiple
> -media maintainers do the review.
> +The authenticity of developers submitting pull requests and merge requests
> +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
>  
> -The media maintainers that work on specific areas of the subsystem are:
> +With the pull request workflow, pull requests shall use a PGP-signed tag.
>  
> -- Remote Controllers (infrared):
> -    Sean Young <sean@mess.org>
> +For more details about PGP sign, please read
> +Documentation/process/maintainer-pgp-guide.rst.

s/.*/:ref:`the PGP guide <pgpguide>`/

>  
> -- HDMI CEC:
> -    Hans Verkuil <hverkuil@xs4all.nl>
> +Subsystem maintainers
> +---------------------
>  
> -- Media controller drivers:
> -    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> -
> -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> -    Sakari Ailus <sakari.ailus@linux.intel.com>
> -
> -- V4L2 drivers and core V4L2 frameworks:
> -    Hans Verkuil <hverkuil@xs4all.nl>
> -
> -The subsystem maintainer is:
> -  Mauro Carvalho Chehab <mchehab@kernel.org>
> -
> -Media maintainers may delegate a patch to other media maintainers as needed.
> -On such case, checkpatch's ``delegate`` field indicates who's currently
> -responsible for reviewing a patch.
> +The subsystem maintainers are:
> +  - Mauro Carvalho Chehab <mchehab@kernel.org> and
> +  - Hans Verkuil <hverkuil@xs4all.nl>
>  
>  Submit Checklist Addendum
>  -------------------------
> @@ -108,17 +225,14 @@ implementing the media APIs:
>  ====================	=======================================================
>  Type			Tool
>  ====================	=======================================================
> -V4L2 drivers\ [3]_	``v4l2-compliance``
> +V4L2 drivers\ [4]_	``v4l2-compliance``
>  V4L2 virtual drivers	``contrib/test/test-media``
>  CEC drivers		``cec-compliance``
>  ====================	=======================================================
>  
> -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
>         V4L2 drivers.
>  
> -Other compilance tools are under development to check other parts of the
> -subsystem.
> -
>  Those tests need to pass before the patches go upstream.
>  
>  Also, please notice that we build the Kernel with::
> @@ -134,6 +248,8 @@ Where the check script is::
>  Be sure to not introduce new warnings on your patches without a
>  very good reason.
>  
> +Please see `Media development workflow`_ for e-mail submission rules.
> +
>  Style Cleanup Patches
>  +++++++++++++++++++++
>  
> @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
>  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 in a couple of weeks or to ask
> -other developers to publicly add Reviewed-by and, more importantly,
> +other developers to publicly add ``Reviewed-by:`` and, more importantly,
>  ``Tested-by:`` tags.
>  
>  Please note that we expect a detailed description for ``Tested-by:``,
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1e930c7a58b1..c77f56a2e695 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
>  M:	Mauro Carvalho Chehab <mchehab@kernel.org>
>  L:	linux-media@vger.kernel.org
>  S:	Maintained
> +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
>  W:	https://linuxtv.org
>  Q:	http://patchwork.kernel.org/project/linux-media/list/
>  T:	git git://linuxtv.org/media.git

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02  9:26 ` [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers Mauro Carvalho Chehab
  2024-12-02 12:40   ` Sakari Ailus
@ 2024-12-02 13:17   ` Hans Verkuil
  2024-12-02 13:54     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 26+ messages in thread
From: Hans Verkuil @ 2024-12-02 13:17 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, linux-media
  Cc: linux-kernel, Hans Verkuil, Ricardo Ribalda

Hi Mauro,

On 02/12/2024 10:26, 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.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>

xs4ll.nl -> xs4all.nl

(it's probably my typo, but please fix this in the final version)

> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
>  MAINTAINERS                                   |   1 +
>  2 files changed, 163 insertions(+), 46 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> index ffc712a5f632..dc764163cf1c 100644
> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
>  Both media userspace and Kernel APIs are documented and the documentation
>  must be kept in sync with the API changes. It means that all patches that
>  add new features to the subsystem must also bring changes to the
> -corresponding API files.
> +corresponding API documentation files.
>  
> -Due to the size and wide scope of the media subsystem, media's
> -maintainership model is to have sub-maintainers that have a broad
> -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> -task to review the patches, providing feedback to users if the patches are
> +Due to the size and wide scope of the media subsystem, the media's
> +maintainership model is to have committers that have a broad knowledge of
> +a specific aspect of the subsystem. It is the committers' task to
> +review the patches, providing feedback to users if the patches are
>  following the subsystem rules and are properly using the media kernel and
>  userspace APIs.
>  
> -Patches for the media subsystem must be sent to the media mailing list
> -at linux-media@vger.kernel.org as plain text only e-mail. Emails with
> -HTML will be automatically rejected by the mail server. It could be wise
> -to also copy the sub-maintainer(s).
> +Media committers
> +----------------
> +
> +In the media subsystem, there are experienced developers who can push
> +patches directly to the development tree. These developers are called
> +Media committers and are divided into the following categories:
> +
> +- Committers:
> +    contributors for one or more drivers within the media subsystem.
> +    They can push changes to the tree that do not affect the core or ABI.
> +
> +- Core committers:
> +    responsible for part of the media core. They are typically
> +    responsible for one or more drivers within the media subsystem, but, besides
> +    that, they can also merge patches that change the code common to multiple
> +    drivers, including the kernel internal API.
> +
> +- Subsystem maintainers:
> +    responsible for the subsystem as a whole, with access to the
> +    entire subsystem.
> +
> +    Only subsystem maintainers can push changes that affect the userspace
> +    API/ABI.
> +
> +All media committers shall explicitly agree with the Kernel development process
> +as described at Documentation/process/index.rst and to the Kernel
> +development rules inside the Kernel documentation, including its code of
> +conduct.
> +
> +Media development tree
> +----------------------
> +
> +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> +where we also maintain news about the subsystem, wiki pages and a patchwork
> +instance where we track patches though their lifetime.
> +
> +The main tree used by media developers is at:
> +
> +https://git.linuxtv.org/media.git/
> +
> +.. _Media development workflow:
> +
> +Media development workflow
> +++++++++++++++++++++++++++
> +
> +All changes for the media subsystem must be sent first as e-mails to the
> +media mailing list, following the process documented at
> +Documentation/process/index.rst.
> +
> +It means that patches shall be submitted as plain text only via e-mail to
> +linux-media@vger.kernel.org. While subscription is not mandatory, you
> +can find details about how to subscribe to it and to see its archives at:
> +
> +  https://subspace.kernel.org/vger.kernel.org.html
> +
> +Emails with HTML will be automatically rejected by the mail server.
> +
> +It could be wise to also copy the media committer(s). You should use
> +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> +Please always copy driver's authors and maintainers.
> +
> +Such patches need to be based against a public branch or tag as follows:
> +
> +1. Patches for new features need to be based at the ``next`` branch of
> +   media.git tree;
> +
> +2. Fixes against an already released kernel should preferably be against
> +   the latest released Kernel. If they require a previously-applied
> +   change at media.git tree, they need to be against its ``fixes`` branch.

What is an "released kernel"? Does an -rcX kernel qualify?

> +
> +3. Fixes for issues not present at the latest released kernel shall
> +   be either against a -rc kernel for an upcoming release or
> +   against the ``fixes`` branch of the media.git tree.

2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
at some point.

> +
> +Patches with fixes shall have:
> +
> +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> +- when applicable, a ``Cc: stable@vger.kernel.org``.
> +
> +Patches that were fixing bugs publicly reported by someone at the
> +linux-media@vger.kernel.org mailing list shall have:
> +
> +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> +
> +Patches that change API shall update documentation accordingly at the
> +same patch series.
> +
> +See Documentation/process/index.rst for more details about e-mail submission.
> +
> +Once a patch is submitted, it may follow either one of the following
> +workflows:
> +
> +a. Pull request workflow: patches are handled by subsystem maintainers::
> +
> +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
> +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> +                +---------+   +-------+   +-----------------------+
> +
> +   For this workflow, pull requests can be generated by a committer,
> +   a previous committer, subsystem maintainers or by a trusted long-time
> +   contributor. If you are not in such group, please don't submit
> +   pull requests, as they will not be processed.
> +
> +b. Committers' workflow: patches are handled by media committers::
> +
> +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> +                +---------+   +--------------------+   +-----------+
> +
> +On both workflows, all patches shall be properly reviewed at
> +linux-media@vger.kernel.org before being merged at media-committers.git.
> +
> +When patches are picked by patchwork and when merged at media-committers,
> +CI bots will check for errors and may provide e-mail feedback about
> +patch problems. When this happens, the patch submitter must fix them, or
> +explain why the errors are false positives.
> +
> +Patches will only be moved to the next stage in those two workflows if they
> +don't fail on CI or if there are false-positives in the CI reports.
> +
> +Failures during e-mail submission
> ++++++++++++++++++++++++++++++++++
>  
>  Media's workflow is heavily based on Patchwork, meaning that, once a patch
>  is submitted, the e-mail will first be accepted by the mailing list
> @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
>  
>     - https://patchwork.linuxtv.org/project/linux-media/list/
>  
> -If it doesn't automatically appear there after a few minutes, then
> +If it doesn't automatically appear there after some time [2]_, then
>  probably something went wrong on your submission. Please check if the
> -email is in plain text\ [2]_ only and if your emailer is not mangling
> +email is in plain text\ [3]_ only and if your emailer is not mangling
>  whitespaces before complaining or submitting them again.
>  
> -You can check if the mailing list server accepted your patch, by looking at:
> +To troubleshoot problems, you should first check if the mailing list
> +server has accepted your patch, by looking at:
>  
>     - https://lore.kernel.org/linux-media/
>  
> -.. [2] If your email contains HTML, the mailing list server will simply
> +If the patch is there and not at patchwork, it is likely that your e-mailer
> +mangled the patch. Patchwork internally has a logic that checks if the

a logic -> logic

> +received e-mail contain a valid patch. Any whitespace and new line

contain -> contains

> +breakages mangling the patch won't be recognized by patchwork, thus such
> +patch will be rejected.
> +
> +.. [2] It usually takes a few minutes for the patch to arrive, but
> +       the e-mail server may be busy, so it may take up to a few hours
> +       for a patch to be picked by patchwork.
> +
> +.. [3] If your email contains HTML, the mailing list server will simply
>         drop it, without any further notice.
>  
> +.. _media-developers-gpg:
>  
> -Media maintainers
> -+++++++++++++++++
> +Authentication for pull and merge requests
> +++++++++++++++++++++++++++++++++++++++++++
>  
> -At the media subsystem, we have a group of senior developers that
> -are responsible for doing the code reviews at the drivers (also known as
> -sub-maintainers), and another senior developer responsible for the
> -subsystem as a whole. For core changes, whenever possible, multiple
> -media maintainers do the review.
> +The authenticity of developers submitting pull requests and merge requests
> +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.

sign -> signing

Hmm, merge requests through gitlab are not signed. So this isn't correct, I
think.

>  
> -The media maintainers that work on specific areas of the subsystem are:
> +With the pull request workflow, pull requests shall use a PGP-signed tag.
>  
> -- Remote Controllers (infrared):
> -    Sean Young <sean@mess.org>
> +For more details about PGP sign, please read
> +Documentation/process/maintainer-pgp-guide.rst.
>  
> -- HDMI CEC:
> -    Hans Verkuil <hverkuil@xs4all.nl>
> +Subsystem maintainers
> +---------------------
>  
> -- Media controller drivers:
> -    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> -
> -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> -    Sakari Ailus <sakari.ailus@linux.intel.com>
> -
> -- V4L2 drivers and core V4L2 frameworks:
> -    Hans Verkuil <hverkuil@xs4all.nl>
> -
> -The subsystem maintainer is:
> -  Mauro Carvalho Chehab <mchehab@kernel.org>
> -
> -Media maintainers may delegate a patch to other media maintainers as needed.
> -On such case, checkpatch's ``delegate`` field indicates who's currently
> -responsible for reviewing a patch.
> +The subsystem maintainers are:
> +  - Mauro Carvalho Chehab <mchehab@kernel.org> and
> +  - Hans Verkuil <hverkuil@xs4all.nl>
>  
>  Submit Checklist Addendum
>  -------------------------
> @@ -108,17 +225,14 @@ implementing the media APIs:
>  ====================	=======================================================
>  Type			Tool
>  ====================	=======================================================
> -V4L2 drivers\ [3]_	``v4l2-compliance``
> +V4L2 drivers\ [4]_	``v4l2-compliance``
>  V4L2 virtual drivers	``contrib/test/test-media``
>  CEC drivers		``cec-compliance``
>  ====================	=======================================================
>  
> -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside

The ``v4l2-compliance`` utility

(add 'utility')

>         V4L2 drivers.
>  
> -Other compilance tools are under development to check other parts of the
> -subsystem.
> -
>  Those tests need to pass before the patches go upstream.
>  
>  Also, please notice that we build the Kernel with::
> @@ -134,6 +248,8 @@ Where the check script is::
>  Be sure to not introduce new warnings on your patches without a
>  very good reason.
>  
> +Please see `Media development workflow`_ for e-mail submission rules.
> +
>  Style Cleanup Patches
>  +++++++++++++++++++++
>  
> @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
>  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 in a couple of weeks or to ask
> -other developers to publicly add Reviewed-by and, more importantly,
> +other developers to publicly add ``Reviewed-by:`` and, more importantly,
>  ``Tested-by:`` tags.
>  
>  Please note that we expect a detailed description for ``Tested-by:``,
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1e930c7a58b1..c77f56a2e695 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
>  M:	Mauro Carvalho Chehab <mchehab@kernel.org>
>  L:	linux-media@vger.kernel.org
>  S:	Maintained
> +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
>  W:	https://linuxtv.org
>  Q:	http://patchwork.kernel.org/project/linux-media/list/

Shouldn't this point to our patchwork instance instead?

>  T:	git git://linuxtv.org/media.git

Regards,

	Hans

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02 12:40   ` Sakari Ailus
@ 2024-12-02 13:23     ` Mauro Carvalho Chehab
  2024-12-03 11:52       ` Sakari Ailus
  0 siblings, 1 reply; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-02 13:23 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, linux-kernel, Hans Verkuil, Ricardo Ribalda

Em Mon, 2 Dec 2024 12:40:44 +0000
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> Hi Mauro,
> 
> Thanks for the set.
> 
> Looks good overall, please still see my comments below.
> 
> On Mon, Dec 02, 2024 at 10:26:19AM +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.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
> > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> >  MAINTAINERS                                   |   1 +
> >  2 files changed, 163 insertions(+), 46 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > index ffc712a5f632..dc764163cf1c 100644
> > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> >  Both media userspace and Kernel APIs are documented and the documentation
> >  must be kept in sync with the API changes. It means that all patches that
> >  add new features to the subsystem must also bring changes to the
> > -corresponding API files.
> > +corresponding API documentation files.
> >  
> > -Due to the size and wide scope of the media subsystem, media's
> > -maintainership model is to have sub-maintainers that have a broad
> > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > -task to review the patches, providing feedback to users if the patches are
> > +Due to the size and wide scope of the media subsystem, the media's
> > +maintainership model is to have committers that have a broad knowledge of
> > +a specific aspect of the subsystem. It is the committers' task to
> > +review the patches, providing feedback to users if the patches are
> >  following the subsystem rules and are properly using the media kernel and
> >  userspace APIs.
> >  
> > -Patches for the media subsystem must be sent to the media mailing list
> > -at linux-media@vger.kernel.org as plain text only e-mail. Emails with
> > -HTML will be automatically rejected by the mail server. It could be wise
> > -to also copy the sub-maintainer(s).
> > +Media committers
> > +----------------
> > +
> > +In the media subsystem, there are experienced developers who can push  
> 
> The media subsystem is generally understood to comprise of what's under
> drivers/media, this should be referring to the community instead.

Do you have a proposal for a different text here?

> 
> > +patches directly to the development tree. These developers are called
> > +Media committers and are divided into the following categories:
> > +
> > +- Committers:
> > +    contributors for one or more drivers within the media subsystem.
> > +    They can push changes to the tree that do not affect the core or ABI.
> > +
> > +- Core committers:
> > +    responsible for part of the media core. They are typically
> > +    responsible for one or more drivers within the media subsystem, but, besides
> > +    that, they can also merge patches that change the code common to multiple
> > +    drivers, including the kernel internal API.
> > +
> > +- Subsystem maintainers:  
> 
> s/Subsystem/Media tree/

Here, we're talking specifically about my hole and Hans, which will
co-maintain the subsystem with me.. I guess subsystem maintainer is the 
best to describe it.

Besides that, we used "media maintainers" in the past with a different
meaning. Better to not re-use it here.
> 
> ?
> > +    responsible for the subsystem as a whole, with access to the
> > +    entire subsystem.
> > +
> > +    Only subsystem maintainers can push changes that affect the userspace
> > +    API/ABI.  
> 
> This is ambiguous. I think it should intend to say API/ABI changes require
> approval from Media tree maintainers.

At the first moment, the idea is to commit them via PRs. So, no such
commits will be merged by committers/core committers, but yeah, it
could also be merged directly by a committer if it has our approval.

In any case, such changes need a consensus from the subsystem maintainers,
which can just be based on a trust relationship between them with
regards to certain parts of the subsystem, or via explicit acks.

Maybe:

	API/ABI changes are done via consensus between subsystem
	maintainers.

	Only subsystem maintainers push changes that affect the userspace
	API/ABI. Committers may push directly if they have approvals
	from subsystem maintainers.

> What constitutes a UAPI change is a topic of discussion on its own. 

Everything that would break backward compatibility with existing non-kernel
code are API/ABI changes.

> Does it
> require adding a new IOCTL? Taking into use a reserved field? Changing
> little-used driver behaviour slightly? 

For all of those, yes: any changes affecting the behavior or fields/nodes
exported via ioctls and sysfs, including new V4L2 controls are API/ABI changes.

Some changes at OF are also API/ABI changes, but those can flow via
committers, provided that OF maintainers added their review or acked-by.

> Fixing a bug in a driver?

No, fixing a bug, even if related to ABI/API non-compliance aren't.

Yet, ABI/API behavior changes at drivers shall not cause regressions.

I don't think we need to let it clear at the text

> The first two obviously yes, but the latter two probably not.
> 
> Also:
> 
> s/Only subsystem maintainers can push/Media tree maintainers' ack is
> required for/
> 
> ?
> 
> > +
> > +All media committers shall explicitly agree with the Kernel development process
> > +as described at Documentation/process/index.rst and to the Kernel
> > +development rules inside the Kernel documentation, including its code of
> > +conduct.
> > +
> > +Media development tree
> > +----------------------
> > +
> > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > +instance where we track patches though their lifetime.
> > +
> > +The main tree used by media developers is at:
> > +
> > +https://git.linuxtv.org/media.git/
> > +
> > +.. _Media development workflow:
> > +
> > +Media development workflow
> > +++++++++++++++++++++++++++
> > +
> > +All changes for the media subsystem must be sent first as e-mails to the
> > +media mailing list, following the process documented at  
> 
> s/^/linux-/
> 
> Also I'd refer to it as "LMML".

We can add an alias there, but better to be explicit about what mailing
list we're referring to.

> 
> > +Documentation/process/index.rst.
> > +
> > +It means that patches shall be submitted as plain text only via e-mail to
> > +linux-media@vger.kernel.org. While subscription is not mandatory, you
> > +can find details about how to subscribe to it and to see its archives at:
> > +
> > +  https://subspace.kernel.org/vger.kernel.org.html
> > +
> > +Emails with HTML will be automatically rejected by the mail server.
> > +
> > +It could be wise to also copy the media committer(s). You should use
> > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > +Please always copy driver's authors and maintainers.
> > +
> > +Such patches need to be based against a public branch or tag as follows:
> > +
> > +1. Patches for new features need to be based at the ``next`` branch of
> > +   media.git tree;
> > +
> > +2. Fixes against an already released kernel should preferably be against
> > +   the latest released Kernel. If they require a previously-applied
> > +   change at media.git tree, they need to be against its ``fixes`` branch.  
> 
> This would be a change to the current process that I don't think has been
> discussed. If I understand correctly, generally this applies to patches
> that have been merged to the development branch (formerly media stage tree
> master) with Cc: stable and Fixes: tags.


Not really a change: this is what we do in practice (except that we renamed
master to next).

We did discuss that during the LPC week. There was a request there about
simplifying the trees during media summit with some suggestions. I did a 
followup meeting with Hans afterwards for us to check what would work best.

The change is, basically:
	media-tree master -> media.git next
	media-tree fixes   -> media.git fixes

	media-stage master -> media-committers next

Subsystem maintainers are also merging patches at media-committers fixes,
in order to let media-ci to test the fixes branch.

As agreed during the media summit, only subsystem maintainers will be
merging fixes patches.

> > +
> > +3. Fixes for issues not present at the latest released kernel shall
> > +   be either against a -rc kernel for an upcoming release or
> > +   against the ``fixes`` branch of the media.git tree.
> > +
> > +Patches with fixes shall have:
> > +
> > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > +- when applicable, a ``Cc: stable@vger.kernel.org``.
> > +
> > +Patches that were fixing bugs publicly reported by someone at the
> > +linux-media@vger.kernel.org mailing list shall have:
> > +
> > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > +
> > +Patches that change API shall update documentation accordingly at the
> > +same patch series.
> > +
> > +See Documentation/process/index.rst for more details about e-mail submission.
> > +
> > +Once a patch is submitted, it may follow either one of the following
> > +workflows:
> > +
> > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > +
> > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|  
> 
> s/e-mail/LMML/

maybe, instead: e-mail to LMML.

> (see earlier comment)? Same below.
> 
> > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > +                +---------+   +-------+   +-----------------------+
> > +
> > +   For this workflow, pull requests can be generated by a committer,
> > +   a previous committer, subsystem maintainers or by a trusted long-time  
> 
> s/previous/former/
> 
> I'd also use plural in all cases here.

Ok.

> 
> > +   contributor. If you are not in such group, please don't submit  
> 
> > +   pull requests, as they will not be processed.
> > +
> > +b. Committers' workflow: patches are handled by media committers::
> > +
> > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > +                +---------+   +--------------------+   +-----------+
> > +
> > +On both workflows, all patches shall be properly reviewed at
> > +linux-media@vger.kernel.org before being merged at media-committers.git.
> > +
> > +When patches are picked by patchwork and when merged at media-committers,
> > +CI bots will check for errors and may provide e-mail feedback about
> > +patch problems. When this happens, the patch submitter must fix them, or
> > +explain why the errors are false positives.
> > +
> > +Patches will only be moved to the next stage in those two workflows if they
> > +don't fail on CI or if there are false-positives in the CI reports.  
> 
> s/don't fail on/pass/

Ok.

> 
> > +
> > +Failures during e-mail submission
> > ++++++++++++++++++++++++++++++++++
> >  
> >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> >  is submitted, the e-mail will first be accepted by the mailing list
> > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> >  
> >     - https://patchwork.linuxtv.org/project/linux-media/list/
> >  
> > -If it doesn't automatically appear there after a few minutes, then
> > +If it doesn't automatically appear there after some time [2]_, then
> >  probably something went wrong on your submission. Please check if the
> > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > +email is in plain text\ [3]_ only and if your emailer is not mangling
> >  whitespaces before complaining or submitting them again.
> >  
> > -You can check if the mailing list server accepted your patch, by looking at:
> > +To troubleshoot problems, you should first check if the mailing list
> > +server has accepted your patch, by looking at:
> >  
> >     - https://lore.kernel.org/linux-media/
> >  
> > -.. [2] If your email contains HTML, the mailing list server will simply
> > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > +mangled the patch. Patchwork internally has a logic that checks if the
> > +received e-mail contain a valid patch. Any whitespace and new line
> > +breakages mangling the patch won't be recognized by patchwork, thus such
> > +patch will be rejected.
> > +
> > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > +       the e-mail server may be busy, so it may take up to a few hours
> > +       for a patch to be picked by patchwork.
> > +
> > +.. [3] If your email contains HTML, the mailing list server will simply
> >         drop it, without any further notice.
> >  
> > +.. _media-developers-gpg:
> >  
> > -Media maintainers
> > -+++++++++++++++++
> > +Authentication for pull and merge requests
> > +++++++++++++++++++++++++++++++++++++++++++
> >  
> > -At the media subsystem, we have a group of senior developers that
> > -are responsible for doing the code reviews at the drivers (also known as
> > -sub-maintainers), and another senior developer responsible for the
> > -subsystem as a whole. For core changes, whenever possible, multiple
> > -media maintainers do the review.
> > +The authenticity of developers submitting pull requests and merge requests
> > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
> >  
> > -The media maintainers that work on specific areas of the subsystem are:
> > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> >  
> > -- Remote Controllers (infrared):
> > -    Sean Young <sean@mess.org>
> > +For more details about PGP sign, please read
> > +Documentation/process/maintainer-pgp-guide.rst.  
> 
> s/.*/:ref:`the PGP guide <pgpguide>`/

No need. A Sphinx plugin does that automatically.

> 
> >  
> > -- HDMI CEC:
> > -    Hans Verkuil <hverkuil@xs4all.nl>
> > +Subsystem maintainers
> > +---------------------
> >  
> > -- Media controller drivers:
> > -    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > -
> > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > -    Sakari Ailus <sakari.ailus@linux.intel.com>
> > -
> > -- V4L2 drivers and core V4L2 frameworks:
> > -    Hans Verkuil <hverkuil@xs4all.nl>
> > -
> > -The subsystem maintainer is:
> > -  Mauro Carvalho Chehab <mchehab@kernel.org>
> > -
> > -Media maintainers may delegate a patch to other media maintainers as needed.
> > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > -responsible for reviewing a patch.
> > +The subsystem maintainers are:
> > +  - Mauro Carvalho Chehab <mchehab@kernel.org> and
> > +  - Hans Verkuil <hverkuil@xs4all.nl>
> >  
> >  Submit Checklist Addendum
> >  -------------------------
> > @@ -108,17 +225,14 @@ implementing the media APIs:
> >  ====================	=======================================================
> >  Type			Tool
> >  ====================	=======================================================
> > -V4L2 drivers\ [3]_	``v4l2-compliance``
> > +V4L2 drivers\ [4]_	``v4l2-compliance``
> >  V4L2 virtual drivers	``contrib/test/test-media``
> >  CEC drivers		``cec-compliance``
> >  ====================	=======================================================
> >  
> > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
> >         V4L2 drivers.
> >  
> > -Other compilance tools are under development to check other parts of the
> > -subsystem.
> > -
> >  Those tests need to pass before the patches go upstream.
> >  
> >  Also, please notice that we build the Kernel with::
> > @@ -134,6 +248,8 @@ Where the check script is::
> >  Be sure to not introduce new warnings on your patches without a
> >  very good reason.
> >  
> > +Please see `Media development workflow`_ for e-mail submission rules.
> > +
> >  Style Cleanup Patches
> >  +++++++++++++++++++++
> >  
> > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> >  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 in a couple of weeks or to ask
> > -other developers to publicly add Reviewed-by and, more importantly,
> > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> >  ``Tested-by:`` tags.
> >  
> >  Please note that we expect a detailed description for ``Tested-by:``,
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1e930c7a58b1..c77f56a2e695 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> >  M:	Mauro Carvalho Chehab <mchehab@kernel.org>
> >  L:	linux-media@vger.kernel.org
> >  S:	Maintained
> > +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
> >  W:	https://linuxtv.org
> >  Q:	http://patchwork.kernel.org/project/linux-media/list/
> >  T:	git git://linuxtv.org/media.git  
> 



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02 13:17   ` Hans Verkuil
@ 2024-12-02 13:54     ` Mauro Carvalho Chehab
  2024-12-02 14:49       ` Hans Verkuil
  2024-12-02 20:48       ` Ricardo Ribalda
  0 siblings, 2 replies; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-02 13:54 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, linux-kernel, Hans Verkuil, Ricardo Ribalda

Em Mon, 2 Dec 2024 14:17:48 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> Hi Mauro,
> 
> On 02/12/2024 10:26, 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.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>  
> 
> xs4ll.nl -> xs4all.nl
> 
> (it's probably my typo, but please fix this in the final version)

Yes, I copied and pasted it. Will fix at the next version.

> 
> > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> >  MAINTAINERS                                   |   1 +
> >  2 files changed, 163 insertions(+), 46 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > index ffc712a5f632..dc764163cf1c 100644
> > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> >  Both media userspace and Kernel APIs are documented and the documentation
> >  must be kept in sync with the API changes. It means that all patches that
> >  add new features to the subsystem must also bring changes to the
> > -corresponding API files.
> > +corresponding API documentation files.
> >  
> > -Due to the size and wide scope of the media subsystem, media's
> > -maintainership model is to have sub-maintainers that have a broad
> > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > -task to review the patches, providing feedback to users if the patches are
> > +Due to the size and wide scope of the media subsystem, the media's
> > +maintainership model is to have committers that have a broad knowledge of
> > +a specific aspect of the subsystem. It is the committers' task to
> > +review the patches, providing feedback to users if the patches are
> >  following the subsystem rules and are properly using the media kernel and
> >  userspace APIs.
> >  
> > -Patches for the media subsystem must be sent to the media mailing list
> > -at linux-media@vger.kernel.org as plain text only e-mail. Emails with
> > -HTML will be automatically rejected by the mail server. It could be wise
> > -to also copy the sub-maintainer(s).
> > +Media committers
> > +----------------
> > +
> > +In the media subsystem, there are experienced developers who can push
> > +patches directly to the development tree. These developers are called
> > +Media committers and are divided into the following categories:
> > +
> > +- Committers:
> > +    contributors for one or more drivers within the media subsystem.
> > +    They can push changes to the tree that do not affect the core or ABI.
> > +
> > +- Core committers:
> > +    responsible for part of the media core. They are typically
> > +    responsible for one or more drivers within the media subsystem, but, besides
> > +    that, they can also merge patches that change the code common to multiple
> > +    drivers, including the kernel internal API.
> > +
> > +- Subsystem maintainers:
> > +    responsible for the subsystem as a whole, with access to the
> > +    entire subsystem.
> > +
> > +    Only subsystem maintainers can push changes that affect the userspace
> > +    API/ABI.
> > +
> > +All media committers shall explicitly agree with the Kernel development process
> > +as described at Documentation/process/index.rst and to the Kernel
> > +development rules inside the Kernel documentation, including its code of
> > +conduct.
> > +
> > +Media development tree
> > +----------------------
> > +
> > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > +instance where we track patches though their lifetime.
> > +
> > +The main tree used by media developers is at:
> > +
> > +https://git.linuxtv.org/media.git/
> > +
> > +.. _Media development workflow:
> > +
> > +Media development workflow
> > +++++++++++++++++++++++++++
> > +
> > +All changes for the media subsystem must be sent first as e-mails to the
> > +media mailing list, following the process documented at
> > +Documentation/process/index.rst.
> > +
> > +It means that patches shall be submitted as plain text only via e-mail to
> > +linux-media@vger.kernel.org. While subscription is not mandatory, you
> > +can find details about how to subscribe to it and to see its archives at:
> > +
> > +  https://subspace.kernel.org/vger.kernel.org.html
> > +
> > +Emails with HTML will be automatically rejected by the mail server.
> > +
> > +It could be wise to also copy the media committer(s). You should use
> > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > +Please always copy driver's authors and maintainers.
> > +
> > +Such patches need to be based against a public branch or tag as follows:
> > +
> > +1. Patches for new features need to be based at the ``next`` branch of
> > +   media.git tree;
> > +
> > +2. Fixes against an already released kernel should preferably be against
> > +   the latest released Kernel. If they require a previously-applied
> > +   change at media.git tree, they need to be against its ``fixes`` branch.  
> 
> What is an "released kernel"? Does an -rcX kernel qualify?
> > +
> > +3. Fixes for issues not present at the latest released kernel shall
> > +   be either against a -rc kernel for an upcoming release or
> > +   against the ``fixes`` branch of the media.git tree.  
> 
> 2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
> at some point.

Some Kernel documents use "mainline release" to refer to Kernel final
releases.

One option to make it clearer would be to add a link to:
	https://www.kernel.org/category/releases.html

And then use the terms:
	- "Mainline release" for v6.10, v6.11, v6.12...
	- "Prepatch release" for -rc1, -rc2, -rc3...

What 2 and means are:

	1. Features for the next mainline release:

	   - baseline shall be media.git ``next`` branch;

	2. Fixes bugs at mainline releases:

	   - baseline shall be the latest mainline release or media.git ``fixes``  
	     if changes depend on a fix already merged;

	3. Fixes for the next mainline release:

	   - baseline shall be a prepatch release or media.git ``fixes``  
	     if changes depend on a fix already merged;

	.. Note:

	   See https://www.kernel.org/category/releases.html for an overview
	   about Kernel release types.
  
Would the above text work best you? If not please propose another
text.

In practice, we'll likely accept other baselines for fixes as
well, for practical reasons. From my side, I don't have any issues
if I need to rebase a tested bugfix patch produced against an
older Kernel send by an occasional contributor, if it only has 
trivial merge conflicts.

> > +Patches with fixes shall have:
> > +
> > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > +- when applicable, a ``Cc: stable@vger.kernel.org``.
> > +
> > +Patches that were fixing bugs publicly reported by someone at the
> > +linux-media@vger.kernel.org mailing list shall have:
> > +
> > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > +
> > +Patches that change API shall update documentation accordingly at the
> > +same patch series.
> > +
> > +See Documentation/process/index.rst for more details about e-mail submission.
> > +
> > +Once a patch is submitted, it may follow either one of the following
> > +workflows:
> > +
> > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > +
> > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
> > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > +                +---------+   +-------+   +-----------------------+
> > +
> > +   For this workflow, pull requests can be generated by a committer,
> > +   a previous committer, subsystem maintainers or by a trusted long-time
> > +   contributor. If you are not in such group, please don't submit
> > +   pull requests, as they will not be processed.
> > +
> > +b. Committers' workflow: patches are handled by media committers::
> > +
> > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > +                +---------+   +--------------------+   +-----------+
> > +
> > +On both workflows, all patches shall be properly reviewed at
> > +linux-media@vger.kernel.org before being merged at media-committers.git.
> > +
> > +When patches are picked by patchwork and when merged at media-committers,
> > +CI bots will check for errors and may provide e-mail feedback about
> > +patch problems. When this happens, the patch submitter must fix them, or
> > +explain why the errors are false positives.
> > +
> > +Patches will only be moved to the next stage in those two workflows if they
> > +don't fail on CI or if there are false-positives in the CI reports.
> > +
> > +Failures during e-mail submission
> > ++++++++++++++++++++++++++++++++++
> >  
> >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> >  is submitted, the e-mail will first be accepted by the mailing list
> > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> >  
> >     - https://patchwork.linuxtv.org/project/linux-media/list/
> >  
> > -If it doesn't automatically appear there after a few minutes, then
> > +If it doesn't automatically appear there after some time [2]_, then
> >  probably something went wrong on your submission. Please check if the
> > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > +email is in plain text\ [3]_ only and if your emailer is not mangling
> >  whitespaces before complaining or submitting them again.
> >  
> > -You can check if the mailing list server accepted your patch, by looking at:
> > +To troubleshoot problems, you should first check if the mailing list
> > +server has accepted your patch, by looking at:
> >  
> >     - https://lore.kernel.org/linux-media/
> >  
> > -.. [2] If your email contains HTML, the mailing list server will simply
> > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > +mangled the patch. Patchwork internally has a logic that checks if the  
> 
> a logic -> logic
> 
> > +received e-mail contain a valid patch. Any whitespace and new line  
> 
> contain -> contains
> 
> > +breakages mangling the patch won't be recognized by patchwork, thus such
> > +patch will be rejected.
> > +
> > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > +       the e-mail server may be busy, so it may take up to a few hours
> > +       for a patch to be picked by patchwork.
> > +
> > +.. [3] If your email contains HTML, the mailing list server will simply
> >         drop it, without any further notice.
> >  
> > +.. _media-developers-gpg:
> >  
> > -Media maintainers
> > -+++++++++++++++++
> > +Authentication for pull and merge requests
> > +++++++++++++++++++++++++++++++++++++++++++
> >  
> > -At the media subsystem, we have a group of senior developers that
> > -are responsible for doing the code reviews at the drivers (also known as
> > -sub-maintainers), and another senior developer responsible for the
> > -subsystem as a whole. For core changes, whenever possible, multiple
> > -media maintainers do the review.
> > +The authenticity of developers submitting pull requests and merge requests
> > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.  
> 
> sign -> signing
> 
> Hmm, merge requests through gitlab are not signed. So this isn't correct, I
> think.

No, but they are authenticated based on gitlab's user ID. The authentication
we'll have is when the new committer sends us an e-mail providing the
gitlab's user ID.

See patch 3, as it contains some changes aiming to better explain it. Once
it get the same people reviewing that also reviewed 1 and 2, I'll fold it.

> 
> >  
> > -The media maintainers that work on specific areas of the subsystem are:
> > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> >  
> > -- Remote Controllers (infrared):
> > -    Sean Young <sean@mess.org>
> > +For more details about PGP sign, please read
> > +Documentation/process/maintainer-pgp-guide.rst.
> >  
> > -- HDMI CEC:
> > -    Hans Verkuil <hverkuil@xs4all.nl>
> > +Subsystem maintainers
> > +---------------------
> >  
> > -- Media controller drivers:
> > -    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > -
> > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > -    Sakari Ailus <sakari.ailus@linux.intel.com>
> > -
> > -- V4L2 drivers and core V4L2 frameworks:
> > -    Hans Verkuil <hverkuil@xs4all.nl>
> > -
> > -The subsystem maintainer is:
> > -  Mauro Carvalho Chehab <mchehab@kernel.org>
> > -
> > -Media maintainers may delegate a patch to other media maintainers as needed.
> > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > -responsible for reviewing a patch.
> > +The subsystem maintainers are:
> > +  - Mauro Carvalho Chehab <mchehab@kernel.org> and
> > +  - Hans Verkuil <hverkuil@xs4all.nl>
> >  
> >  Submit Checklist Addendum
> >  -------------------------
> > @@ -108,17 +225,14 @@ implementing the media APIs:
> >  ====================	=======================================================
> >  Type			Tool
> >  ====================	=======================================================
> > -V4L2 drivers\ [3]_	``v4l2-compliance``
> > +V4L2 drivers\ [4]_	``v4l2-compliance``
> >  V4L2 virtual drivers	``contrib/test/test-media``
> >  CEC drivers		``cec-compliance``
> >  ====================	=======================================================
> >  
> > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside  
> 
> The ``v4l2-compliance`` utility
> 
> (add 'utility')

Ok.

> 
> >         V4L2 drivers.
> >  
> > -Other compilance tools are under development to check other parts of the
> > -subsystem.
> > -
> >  Those tests need to pass before the patches go upstream.
> >  
> >  Also, please notice that we build the Kernel with::
> > @@ -134,6 +248,8 @@ Where the check script is::
> >  Be sure to not introduce new warnings on your patches without a
> >  very good reason.
> >  
> > +Please see `Media development workflow`_ for e-mail submission rules.
> > +
> >  Style Cleanup Patches
> >  +++++++++++++++++++++
> >  
> > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> >  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 in a couple of weeks or to ask
> > -other developers to publicly add Reviewed-by and, more importantly,
> > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> >  ``Tested-by:`` tags.
> >  
> >  Please note that we expect a detailed description for ``Tested-by:``,
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1e930c7a58b1..c77f56a2e695 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> >  M:	Mauro Carvalho Chehab <mchehab@kernel.org>
> >  L:	linux-media@vger.kernel.org
> >  S:	Maintained
> > +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
> >  W:	https://linuxtv.org
> >  Q:	http://patchwork.kernel.org/project/linux-media/list/  
> 
> Shouldn't this point to our patchwork instance instead?

Good catch! I'll update it.

> 
> >  T:	git git://linuxtv.org/media.git  
> 
> Regards,
> 
> 	Hans



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 2/3] docs: media: document media multi-committers rules and process
  2024-12-02  9:26 ` [PATCH v3 2/3] docs: media: document media multi-committers rules and process Mauro Carvalho Chehab
@ 2024-12-02 14:13   ` Sakari Ailus
  2024-12-03  8:26     ` Mauro Carvalho Chehab
  2024-12-02 14:37   ` Hans Verkuil
  1 sibling, 1 reply; 26+ messages in thread
From: Sakari Ailus @ 2024-12-02 14:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, Jonathan Corbet, linux-doc, linux-kernel, workflows,
	Hans Verkuil, Ricardo Ribalda

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@kernel.org>
> Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.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@linuxtv.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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 2/3] docs: media: document media multi-committers rules and process
  2024-12-02  9:26 ` [PATCH v3 2/3] docs: media: document media multi-committers rules and process Mauro Carvalho Chehab
  2024-12-02 14:13   ` Sakari Ailus
@ 2024-12-02 14:37   ` Hans Verkuil
  1 sibling, 0 replies; 26+ messages in thread
From: Hans Verkuil @ 2024-12-02 14:37 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, linux-media
  Cc: Jonathan Corbet, linux-doc, linux-kernel, workflows, Hans Verkuil,
	Ricardo Ribalda

On 02/12/2024 10:26, 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@kernel.org>
> Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>

Same typo in my email.

> Reviewed-by: Ricardo Ribalda <ribalda@chromium.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

commiters -> committers

> +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

by -> by the

> +tree, after receiving the e-mail documented at

at -> in

> +: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?
> +--------------------------
> +
> +A media committer is a developer who can push patches from other developers
> +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:
> +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
> +is also based on a trust relationship between the rest of the committers,
> +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;
> +   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.
> +
> +[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
> +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

LinuxTV -> Linux Media

Please do a search-and-replace.

> +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
> +
> +  - the Media development profile:
> +    Documentation/driver-api/media/maintainer-entry-profile.rst
> +
> +- 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
> +at the yearly Linux Media Summit, typically co-located with another Linux
> +conference.
> +
> +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.
> +
> +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

I'd simplify this and just say: all communication related to changing commit rights
should happen in private as much as possible.

> +   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@linuxtv.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

commiter -> committer (do a search if there are more occurrences of the same typo)

> +   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

Such -> That

> +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

authentity -> authenticity

> +media-committer.git tree.
> +
> +In case the kernel development process changes, by merging new commits
> +in the

in -> to

> +`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
> +      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.

I wonder if this short paragraph doesn't belong to the "Media committer's agreement"
section above. It's a weird thing to talk about here, you would expect it in
the section talking about commit rights.

> +
> +.. 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.

committer's grants -> the commit rights

> +
> +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

committer grants -> commit rights

> +accordingly. If you wish to resume contributing later on, then contact
> +the subsystem maintainers to ask if your rights can be restored.

rights -> commit rights

> +
> +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
>  --------------------------------------------
>  

Regards,

	Hans

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties
  2024-12-02  9:26 ` [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties Mauro Carvalho Chehab
@ 2024-12-02 14:40   ` Hans Verkuil
  2024-12-02 14:48   ` Sakari Ailus
  1 sibling, 0 replies; 26+ messages in thread
From: Hans Verkuil @ 2024-12-02 14:40 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, linux-media; +Cc: linux-kernel

On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
> During the review of the media committes profile, it was noticed
> that the responsibility for timely review patches was not clear:
> such review is expected that all developers listed at MAINTAINERS
> with the "M:" tag (e.g. "maintainers" on its broad sense).
> 
> This is orthogonal of being a media committer or not. Such duty
> is implied at:
> 
> 	Documentation/admin-guide/reporting-issues.rst
> 
> and at the MAINTAINERS header, when it says that even when the
> status is "odd fixes", the patches will flow in.
> 
> So, let make it explicit at the maintainer-entry-profile that
> maintainers need to do timely reviews.
> 
> Also, while right now our focus is on granting committer rights to
> maintainers, the media-committer model may evolve in the future to
> accept other committers that don't have such duties.
> 
> So, make it clear at the media-committer.rst that the duties
> related to reviewing patches from others are for the drivers
> they are maintainers as well.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  Documentation/driver-api/media/maintainer-entry-profile.rst | 5 +++++
>  Documentation/driver-api/media/media-committer.rst          | 6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> index 705209eacf58..50568c744129 100644
> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> @@ -153,6 +153,11 @@ b. Committers' workflow: patches are handled by media committers::
>  On both workflows, all patches shall be properly reviewed at
>  linux-media@vger.kernel.org before being merged at media-committers.git.
>  
> +Such patches will be timely-reviewed by developers listed as maintainers at

at -> in

> +the MAINTAINERS file. Such maintainers will follow one of the above
> +workflows, e. g. they will either send a pull request or merge patches

e. g. -> e.g.

> +directly at the media-committers tree.
> +
>  When patches are picked by patchwork and when merged at media-committers,
>  CI bots will check for errors and may provide e-mail feedback about
>  patch problems. When this happens, the patch submitter must fix them, or
> diff --git a/Documentation/driver-api/media/media-committer.rst b/Documentation/driver-api/media/media-committer.rst
> index 3c2f8f413307..ec81f01db126 100644
> --- a/Documentation/driver-api/media/media-committer.rst
> +++ b/Documentation/driver-api/media/media-committer.rst
> @@ -87,9 +87,9 @@ be delegating part of their maintenance tasks.
>  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.
> +and to properly review patches from third parties for the drivers they are

they are maintainers -> that they maintain

> +maintainers in a timely manner and keeping the status of the reviewed code

reviewed code -> patches

> +at https://patchwork.linuxtv.org updated.
>  
>  .. Note::
>  

Regards,

	Hans

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties
  2024-12-02  9:26 ` [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties Mauro Carvalho Chehab
  2024-12-02 14:40   ` Hans Verkuil
@ 2024-12-02 14:48   ` Sakari Ailus
  2024-12-03  9:28     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 26+ messages in thread
From: Sakari Ailus @ 2024-12-02 14:48 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, linux-kernel

Hi Mauro,

On Mon, Dec 02, 2024 at 10:26:21AM +0100, Mauro Carvalho Chehab wrote:
> During the review of the media committes profile, it was noticed

s/committe\K/r/

> that the responsibility for timely review patches was not clear:
> such review is expected that all developers listed at MAINTAINERS
> with the "M:" tag (e.g. "maintainers" on its broad sense).
> 
> This is orthogonal of being a media committer or not. Such duty
> is implied at:
> 
> 	Documentation/admin-guide/reporting-issues.rst
> 
> and at the MAINTAINERS header, when it says that even when the
> status is "odd fixes", the patches will flow in.
> 
> So, let make it explicit at the maintainer-entry-profile that
> maintainers need to do timely reviews.
> 
> Also, while right now our focus is on granting committer rights to
> maintainers, the media-committer model may evolve in the future to
> accept other committers that don't have such duties.
> 
> So, make it clear at the media-committer.rst that the duties
> related to reviewing patches from others are for the drivers
> they are maintainers as well.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  Documentation/driver-api/media/maintainer-entry-profile.rst | 5 +++++
>  Documentation/driver-api/media/media-committer.rst          | 6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> index 705209eacf58..50568c744129 100644
> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> @@ -153,6 +153,11 @@ b. Committers' workflow: patches are handled by media committers::
>  On both workflows, all patches shall be properly reviewed at
>  linux-media@vger.kernel.org before being merged at media-committers.git.
>  
> +Such patches will be timely-reviewed by developers listed as maintainers at
> +the MAINTAINERS file. Such maintainers will follow one of the above

I'd put this as:

Such patches will be reviewed timely by the maintainers and reviewers as
listed in the MAINTAINERS file.

> +workflows, e. g. they will either send a pull request or merge patches
> +directly at the media-committers tree.

Can we expect people listed as maintainers to either send PRs or be media
committers? I think this might be eventually the result but I think we're
quite far from this currently and I expect things to remain that way in the
near future.


> +
>  When patches are picked by patchwork and when merged at media-committers,
>  CI bots will check for errors and may provide e-mail feedback about
>  patch problems. When this happens, the patch submitter must fix them, or
> diff --git a/Documentation/driver-api/media/media-committer.rst b/Documentation/driver-api/media/media-committer.rst
> index 3c2f8f413307..ec81f01db126 100644
> --- a/Documentation/driver-api/media/media-committer.rst
> +++ b/Documentation/driver-api/media/media-committer.rst
> @@ -87,9 +87,9 @@ be delegating part of their maintenance tasks.
>  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.
> +and to properly review patches from third parties for the drivers they are
> +maintainers in a timely manner and keeping the status of the reviewed code

s/code/patches/

> +at https://patchwork.linuxtv.org updated.
>  
>  .. Note::
>  

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02 13:54     ` Mauro Carvalho Chehab
@ 2024-12-02 14:49       ` Hans Verkuil
  2024-12-02 20:48       ` Ricardo Ribalda
  1 sibling, 0 replies; 26+ messages in thread
From: Hans Verkuil @ 2024-12-02 14:49 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Hans Verkuil, Ricardo Ribalda

On 02/12/2024 14:54, Mauro Carvalho Chehab wrote:
> Em Mon, 2 Dec 2024 14:17:48 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> Hi Mauro,
>>
>> On 02/12/2024 10:26, 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.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>>> Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>  
>>
>> xs4ll.nl -> xs4all.nl
>>
>> (it's probably my typo, but please fix this in the final version)
> 
> Yes, I copied and pasted it. Will fix at the next version.
> 
>>
>>> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>>> ---
>>>  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
>>>  MAINTAINERS                                   |   1 +
>>>  2 files changed, 163 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
>>> index ffc712a5f632..dc764163cf1c 100644
>>> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
>>> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
>>> @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
>>>  Both media userspace and Kernel APIs are documented and the documentation
>>>  must be kept in sync with the API changes. It means that all patches that
>>>  add new features to the subsystem must also bring changes to the
>>> -corresponding API files.
>>> +corresponding API documentation files.
>>>  
>>> -Due to the size and wide scope of the media subsystem, media's
>>> -maintainership model is to have sub-maintainers that have a broad
>>> -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
>>> -task to review the patches, providing feedback to users if the patches are
>>> +Due to the size and wide scope of the media subsystem, the media's
>>> +maintainership model is to have committers that have a broad knowledge of
>>> +a specific aspect of the subsystem. It is the committers' task to
>>> +review the patches, providing feedback to users if the patches are
>>>  following the subsystem rules and are properly using the media kernel and
>>>  userspace APIs.
>>>  
>>> -Patches for the media subsystem must be sent to the media mailing list
>>> -at linux-media@vger.kernel.org as plain text only e-mail. Emails with
>>> -HTML will be automatically rejected by the mail server. It could be wise
>>> -to also copy the sub-maintainer(s).
>>> +Media committers
>>> +----------------
>>> +
>>> +In the media subsystem, there are experienced developers who can push
>>> +patches directly to the development tree. These developers are called
>>> +Media committers and are divided into the following categories:
>>> +
>>> +- Committers:
>>> +    contributors for one or more drivers within the media subsystem.
>>> +    They can push changes to the tree that do not affect the core or ABI.
>>> +
>>> +- Core committers:
>>> +    responsible for part of the media core. They are typically
>>> +    responsible for one or more drivers within the media subsystem, but, besides
>>> +    that, they can also merge patches that change the code common to multiple
>>> +    drivers, including the kernel internal API.
>>> +
>>> +- Subsystem maintainers:
>>> +    responsible for the subsystem as a whole, with access to the
>>> +    entire subsystem.
>>> +
>>> +    Only subsystem maintainers can push changes that affect the userspace
>>> +    API/ABI.
>>> +
>>> +All media committers shall explicitly agree with the Kernel development process
>>> +as described at Documentation/process/index.rst and to the Kernel
>>> +development rules inside the Kernel documentation, including its code of
>>> +conduct.
>>> +
>>> +Media development tree
>>> +----------------------
>>> +
>>> +The main development tree used by the media subsystem is hosted at LinuxTV.org,
>>> +where we also maintain news about the subsystem, wiki pages and a patchwork
>>> +instance where we track patches though their lifetime.
>>> +
>>> +The main tree used by media developers is at:
>>> +
>>> +https://git.linuxtv.org/media.git/
>>> +
>>> +.. _Media development workflow:
>>> +
>>> +Media development workflow
>>> +++++++++++++++++++++++++++
>>> +
>>> +All changes for the media subsystem must be sent first as e-mails to the
>>> +media mailing list, following the process documented at
>>> +Documentation/process/index.rst.
>>> +
>>> +It means that patches shall be submitted as plain text only via e-mail to
>>> +linux-media@vger.kernel.org. While subscription is not mandatory, you
>>> +can find details about how to subscribe to it and to see its archives at:
>>> +
>>> +  https://subspace.kernel.org/vger.kernel.org.html
>>> +
>>> +Emails with HTML will be automatically rejected by the mail server.
>>> +
>>> +It could be wise to also copy the media committer(s). You should use
>>> +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
>>> +Please always copy driver's authors and maintainers.
>>> +
>>> +Such patches need to be based against a public branch or tag as follows:
>>> +
>>> +1. Patches for new features need to be based at the ``next`` branch of
>>> +   media.git tree;
>>> +
>>> +2. Fixes against an already released kernel should preferably be against
>>> +   the latest released Kernel. If they require a previously-applied
>>> +   change at media.git tree, they need to be against its ``fixes`` branch.  
>>
>> What is an "released kernel"? Does an -rcX kernel qualify?
>>> +
>>> +3. Fixes for issues not present at the latest released kernel shall
>>> +   be either against a -rc kernel for an upcoming release or
>>> +   against the ``fixes`` branch of the media.git tree.  
>>
>> 2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
>> at some point.
> 
> Some Kernel documents use "mainline release" to refer to Kernel final
> releases.
> 
> One option to make it clearer would be to add a link to:
> 	https://www.kernel.org/category/releases.html
> 
> And then use the terms:
> 	- "Mainline release" for v6.10, v6.11, v6.12...
> 	- "Prepatch release" for -rc1, -rc2, -rc3...
> 
> What 2 and means are:
> 
> 	1. Features for the next mainline release:
> 
> 	   - baseline shall be media.git ``next`` branch;
> 
> 	2. Fixes bugs at mainline releases:
> 
> 	   - baseline shall be the latest mainline release or media.git ``fixes``  
> 	     if changes depend on a fix already merged;
> 
> 	3. Fixes for the next mainline release:
> 
> 	   - baseline shall be a prepatch release or media.git ``fixes``  
> 	     if changes depend on a fix already merged;

If it is for the next mainline release, then I would also be fine with 'next'
as baseline. That avoids having to modify the patch if it conflicts with
new work that went into next. Having to base a fix on the 'fixes' branch is
rare in this case. You typically only need to do that in case 2.

> 
> 	.. Note:
> 
> 	   See https://www.kernel.org/category/releases.html for an overview
> 	   about Kernel release types.
>   
> Would the above text work best you? If not please propose another
> text.

Yes, that's much better.

> 
> In practice, we'll likely accept other baselines for fixes as
> well, for practical reasons. From my side, I don't have any issues
> if I need to rebase a tested bugfix patch produced against an
> older Kernel send by an occasional contributor, if it only has 
> trivial merge conflicts.

Sure, these three points are really about our preference since following
that reduces the chance of merge conflicts.

We can say something like this:

"To minimize the chance of merge conflicts for your patch series, we recommend
that you use the following baseline for your patch series:"

> 
>>> +Patches with fixes shall have:
>>> +
>>> +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
>>> +- when applicable, a ``Cc: stable@vger.kernel.org``.
>>> +
>>> +Patches that were fixing bugs publicly reported by someone at the
>>> +linux-media@vger.kernel.org mailing list shall have:
>>> +
>>> +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
>>> +
>>> +Patches that change API shall update documentation accordingly at the
>>> +same patch series.
>>> +
>>> +See Documentation/process/index.rst for more details about e-mail submission.
>>> +
>>> +Once a patch is submitted, it may follow either one of the following
>>> +workflows:
>>> +
>>> +a. Pull request workflow: patches are handled by subsystem maintainers::
>>> +
>>> +     +------+   +---------+   +-------+   +-----------------------+   +---------+
>>> +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
>>> +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
>>> +                +---------+   +-------+   +-----------------------+
>>> +
>>> +   For this workflow, pull requests can be generated by a committer,
>>> +   a previous committer, subsystem maintainers or by a trusted long-time
>>> +   contributor. If you are not in such group, please don't submit
>>> +   pull requests, as they will not be processed.
>>> +
>>> +b. Committers' workflow: patches are handled by media committers::
>>> +
>>> +     +------+   +---------+   +--------------------+   +-----------+   +---------+
>>> +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
>>> +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
>>> +                +---------+   +--------------------+   +-----------+
>>> +
>>> +On both workflows, all patches shall be properly reviewed at
>>> +linux-media@vger.kernel.org before being merged at media-committers.git.
>>> +
>>> +When patches are picked by patchwork and when merged at media-committers,
>>> +CI bots will check for errors and may provide e-mail feedback about
>>> +patch problems. When this happens, the patch submitter must fix them, or
>>> +explain why the errors are false positives.
>>> +
>>> +Patches will only be moved to the next stage in those two workflows if they
>>> +don't fail on CI or if there are false-positives in the CI reports.
>>> +
>>> +Failures during e-mail submission
>>> ++++++++++++++++++++++++++++++++++
>>>  
>>>  Media's workflow is heavily based on Patchwork, meaning that, once a patch
>>>  is submitted, the e-mail will first be accepted by the mailing list
>>> @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
>>>  
>>>     - https://patchwork.linuxtv.org/project/linux-media/list/
>>>  
>>> -If it doesn't automatically appear there after a few minutes, then
>>> +If it doesn't automatically appear there after some time [2]_, then
>>>  probably something went wrong on your submission. Please check if the
>>> -email is in plain text\ [2]_ only and if your emailer is not mangling
>>> +email is in plain text\ [3]_ only and if your emailer is not mangling
>>>  whitespaces before complaining or submitting them again.
>>>  
>>> -You can check if the mailing list server accepted your patch, by looking at:
>>> +To troubleshoot problems, you should first check if the mailing list
>>> +server has accepted your patch, by looking at:
>>>  
>>>     - https://lore.kernel.org/linux-media/
>>>  
>>> -.. [2] If your email contains HTML, the mailing list server will simply
>>> +If the patch is there and not at patchwork, it is likely that your e-mailer
>>> +mangled the patch. Patchwork internally has a logic that checks if the  
>>
>> a logic -> logic
>>
>>> +received e-mail contain a valid patch. Any whitespace and new line  
>>
>> contain -> contains
>>
>>> +breakages mangling the patch won't be recognized by patchwork, thus such
>>> +patch will be rejected.
>>> +
>>> +.. [2] It usually takes a few minutes for the patch to arrive, but
>>> +       the e-mail server may be busy, so it may take up to a few hours
>>> +       for a patch to be picked by patchwork.
>>> +
>>> +.. [3] If your email contains HTML, the mailing list server will simply
>>>         drop it, without any further notice.
>>>  
>>> +.. _media-developers-gpg:
>>>  
>>> -Media maintainers
>>> -+++++++++++++++++
>>> +Authentication for pull and merge requests
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>  
>>> -At the media subsystem, we have a group of senior developers that
>>> -are responsible for doing the code reviews at the drivers (also known as
>>> -sub-maintainers), and another senior developer responsible for the
>>> -subsystem as a whole. For core changes, whenever possible, multiple
>>> -media maintainers do the review.
>>> +The authenticity of developers submitting pull requests and merge requests
>>> +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.  
>>
>> sign -> signing
>>
>> Hmm, merge requests through gitlab are not signed. So this isn't correct, I
>> think.
> 
> No, but they are authenticated based on gitlab's user ID. The authentication
> we'll have is when the new committer sends us an e-mail providing the
> gitlab's user ID.
> 
> See patch 3, as it contains some changes aiming to better explain it. Once
> it get the same people reviewing that also reviewed 1 and 2, I'll fold it.
> 
>>
>>>  
>>> -The media maintainers that work on specific areas of the subsystem are:
>>> +With the pull request workflow, pull requests shall use a PGP-signed tag.
>>>  
>>> -- Remote Controllers (infrared):
>>> -    Sean Young <sean@mess.org>
>>> +For more details about PGP sign, please read
>>> +Documentation/process/maintainer-pgp-guide.rst.
>>>  
>>> -- HDMI CEC:
>>> -    Hans Verkuil <hverkuil@xs4all.nl>
>>> +Subsystem maintainers
>>> +---------------------
>>>  
>>> -- Media controller drivers:
>>> -    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> -
>>> -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
>>> -    Sakari Ailus <sakari.ailus@linux.intel.com>
>>> -
>>> -- V4L2 drivers and core V4L2 frameworks:
>>> -    Hans Verkuil <hverkuil@xs4all.nl>
>>> -
>>> -The subsystem maintainer is:
>>> -  Mauro Carvalho Chehab <mchehab@kernel.org>
>>> -
>>> -Media maintainers may delegate a patch to other media maintainers as needed.
>>> -On such case, checkpatch's ``delegate`` field indicates who's currently
>>> -responsible for reviewing a patch.
>>> +The subsystem maintainers are:
>>> +  - Mauro Carvalho Chehab <mchehab@kernel.org> and
>>> +  - Hans Verkuil <hverkuil@xs4all.nl>
>>>  
>>>  Submit Checklist Addendum
>>>  -------------------------
>>> @@ -108,17 +225,14 @@ implementing the media APIs:
>>>  ====================	=======================================================
>>>  Type			Tool
>>>  ====================	=======================================================
>>> -V4L2 drivers\ [3]_	``v4l2-compliance``
>>> +V4L2 drivers\ [4]_	``v4l2-compliance``
>>>  V4L2 virtual drivers	``contrib/test/test-media``
>>>  CEC drivers		``cec-compliance``
>>>  ====================	=======================================================
>>>  
>>> -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
>>> +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside  
>>
>> The ``v4l2-compliance`` utility
>>
>> (add 'utility')
> 
> Ok.
> 
>>
>>>         V4L2 drivers.
>>>  
>>> -Other compilance tools are under development to check other parts of the
>>> -subsystem.
>>> -
>>>  Those tests need to pass before the patches go upstream.
>>>  
>>>  Also, please notice that we build the Kernel with::
>>> @@ -134,6 +248,8 @@ Where the check script is::
>>>  Be sure to not introduce new warnings on your patches without a
>>>  very good reason.
>>>  
>>> +Please see `Media development workflow`_ for e-mail submission rules.
>>> +
>>>  Style Cleanup Patches
>>>  +++++++++++++++++++++
>>>  
>>> @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
>>>  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 in a couple of weeks or to ask
>>> -other developers to publicly add Reviewed-by and, more importantly,
>>> +other developers to publicly add ``Reviewed-by:`` and, more importantly,
>>>  ``Tested-by:`` tags.
>>>  
>>>  Please note that we expect a detailed description for ``Tested-by:``,
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 1e930c7a58b1..c77f56a2e695 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
>>>  M:	Mauro Carvalho Chehab <mchehab@kernel.org>
>>>  L:	linux-media@vger.kernel.org
>>>  S:	Maintained
>>> +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
>>>  W:	https://linuxtv.org
>>>  Q:	http://patchwork.kernel.org/project/linux-media/list/  
>>
>> Shouldn't this point to our patchwork instance instead?
> 
> Good catch! I'll update it.
> 
>>
>>>  T:	git git://linuxtv.org/media.git  

Regards,

	Hans

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 0/3] Document the new media-committer's model
  2024-12-02  9:26 [PATCH v3 0/3] Document the new media-committer's model Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2024-12-02  9:26 ` [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties Mauro Carvalho Chehab
@ 2024-12-02 15:03 ` Hans Verkuil
  2024-12-03  7:19   ` Mauro Carvalho Chehab
  3 siblings, 1 reply; 26+ messages in thread
From: Hans Verkuil @ 2024-12-02 15:03 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, linux-media
  Cc: Jonathan Corbet, linux-doc, linux-kernel, workflows

On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
> The media subsystem used to have a multi-commiter's model in the
> past, but things didn't go well on that time, and we had to move to
> a centralized model.
> 
> As the community has evolved, and as there are now new policies in
> place like CoC, let's experiment with a multi-committers again.
> 
> The model we're using was inspired by the DRM multi-committers
> model. Yet, media subsystem is different on several aspects, so the
> model is not exactly the same.
> 
> The implementation will be in phases. For this phase, the goal is that 
> all committers will be people listed at MAINTAINERS.
> 
> On this series,:
> 
> patch 1: updates the  media maintainer's entry profile and adds the
> workflow that will be used with the new model. While here, it also
> adds a missing "P:" tag at the MAINTAINERS file, pointing to it;
> 
> patch 2: adds a new document focused at the new maintainers
> process. Its target is for developers that will be granted with
> commit rights at the new media-maintainers.git tree. It also
> contains a reference tag addition to kernel.org PGP chain
> at process/maintainer-pgp-guide.rst.
> 
> patch 3: make documents cleared about maintainership duties.

At least from my perspective, v3 is close to being ready and I hope
that v4 will be good enough to be merged.

That said, what is missing in all this is that there is nothing here
that explains why you would want to become a media committer. It is all
very dry stuff, lots of 'shall's, and 'rights' and 'trust' and obligations,
but nothing about the satisfaction you get when you get the responsibility
of a part of the kernel and being able to guide the development of that
area.

It's good enough to get the multi-committer process off the ground, but
it definitely needs more work to make it more inviting to become a media
committer. Because right now it is as dry as dust.

Regards,

	Hans

> 
> ---
> 
> v3:
> - added patch 3;
> - addressed nits pointed by Ricardo during his review;
> - did minor editorial changes to improve Sphinx html output.
> 
> v2: I tried to address most of the suggestions where there was an agreement
> from Laurent's review and further comments. As there were several changes,
> on separate threads, I could have missed something.
> 
> 
> Mauro Carvalho Chehab (3):
>   docs: media: update maintainer-entry-profile for multi-committers
>   docs: media: document media multi-committers rules and process
>   docs: media: profile: make it clearer about maintainership duties
> 
>  Documentation/driver-api/media/index.rst      |   1 +
>  .../media/maintainer-entry-profile.rst        | 219 +++++++++++---
>  .../driver-api/media/media-committer.rst      | 278 ++++++++++++++++++
>  .../process/maintainer-pgp-guide.rst          |   2 +
>  MAINTAINERS                                   |   1 +
>  5 files changed, 456 insertions(+), 45 deletions(-)
>  create mode 100644 Documentation/driver-api/media/media-committer.rst
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02 13:54     ` Mauro Carvalho Chehab
  2024-12-02 14:49       ` Hans Verkuil
@ 2024-12-02 20:48       ` Ricardo Ribalda
  1 sibling, 0 replies; 26+ messages in thread
From: Ricardo Ribalda @ 2024-12-02 20:48 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hans Verkuil, linux-media, linux-kernel, Hans Verkuil

Minor nits that can be ignored

On Mon, 2 Dec 2024 at 14:54, Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Mon, 2 Dec 2024 14:17:48 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
> > Hi Mauro,
> >
> > On 02/12/2024 10:26, 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.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
> >
> > xs4ll.nl -> xs4all.nl
> >
> > (it's probably my typo, but please fix this in the final version)
>
> Yes, I copied and pasted it. Will fix at the next version.
>
> >
> > > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> > >  MAINTAINERS                                   |   1 +
> > >  2 files changed, 163 insertions(+), 46 deletions(-)
> > >
> > > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > index ffc712a5f632..dc764163cf1c 100644
> > > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> > >  Both media userspace and Kernel APIs are documented and the documentation
> > >  must be kept in sync with the API changes. It means that all patches that
> > >  add new features to the subsystem must also bring changes to the
> > > -corresponding API files.
> > > +corresponding API documentation files.
> > >
> > > -Due to the size and wide scope of the media subsystem, media's
> > > -maintainership model is to have sub-maintainers that have a broad
> > > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > > -task to review the patches, providing feedback to users if the patches are
> > > +Due to the size and wide scope of the media subsystem, the media's
> > > +maintainership model is to have committers that have a broad knowledge of
> > > +a specific aspect of the subsystem. It is the committers' task to
> > > +review the patches, providing feedback to users if the patches are
> > >  following the subsystem rules and are properly using the media kernel and
> > >  userspace APIs.
> > >
> > > -Patches for the media subsystem must be sent to the media mailing list
> > > -at linux-media@vger.kernel.org as plain text only e-mail. Emails with
> > > -HTML will be automatically rejected by the mail server. It could be wise
> > > -to also copy the sub-maintainer(s).
> > > +Media committers
> > > +----------------
> > > +
> > > +In the media subsystem, there are experienced developers who can push
> > > +patches directly to the development tree. These developers are called
> > > +Media committers and are divided into the following categories:
> > > +
> > > +- Committers:
> > > +    contributors for one or more drivers within the media subsystem.
> > > +    They can push changes to the tree that do not affect the core or ABI.
> > > +
> > > +- Core committers:
> > > +    responsible for part of the media core. They are typically
> > > +    responsible for one or more drivers within the media subsystem, but, besides
> > > +    that, they can also merge patches that change the code common to multiple
> > > +    drivers, including the kernel internal API.
> > > +
> > > +- Subsystem maintainers:
> > > +    responsible for the subsystem as a whole, with access to the
> > > +    entire subsystem.
> > > +
> > > +    Only subsystem maintainers can push changes that affect the userspace
> > > +    API/ABI.
> > > +
> > > +All media committers shall explicitly agree with the Kernel development process
> > > +as described at Documentation/process/index.rst and to the Kernel
> > > +development rules inside the Kernel documentation, including its code of
> > > +conduct.
> > > +
> > > +Media development tree
> > > +----------------------
> > > +
> > > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > > +instance where we track patches though their lifetime.
> > > +
> > > +The main tree used by media developers is at:
> > > +
> > > +https://git.linuxtv.org/media.git/
> > > +
> > > +.. _Media development workflow:
> > > +
> > > +Media development workflow
> > > +++++++++++++++++++++++++++
> > > +
> > > +All changes for the media subsystem must be sent first as e-mails to the
> > > +media mailing list, following the process documented at
> > > +Documentation/process/index.rst.
> > > +
> > > +It means that patches shall be submitted as plain text only via e-mail to
> > > +linux-media@vger.kernel.org. While subscription is not mandatory, you
> > > +can find details about how to subscribe to it and to see its archives at:
> > > +
> > > +  https://subspace.kernel.org/vger.kernel.org.html
> > > +
> > > +Emails with HTML will be automatically rejected by the mail server.
> > > +
> > > +It could be wise to also copy the media committer(s). You should use
> > > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > > +Please always copy driver's authors and maintainers.
> > > +
> > > +Such patches need to be based against a public branch or tag as follows:
> > > +
> > > +1. Patches for new features need to be based at the ``next`` branch of
> > > +   media.git tree;
> > > +
> > > +2. Fixes against an already released kernel should preferably be against
> > > +   the latest released Kernel. If they require a previously-applied
> > > +   change at media.git tree, they need to be against its ``fixes`` branch.
> >
> > What is an "released kernel"? Does an -rcX kernel qualify?
> > > +
> > > +3. Fixes for issues not present at the latest released kernel shall
> > > +   be either against a -rc kernel for an upcoming release or
> > > +   against the ``fixes`` branch of the media.git tree.
> >
> > 2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
> > at some point.
>
> Some Kernel documents use "mainline release" to refer to Kernel final
> releases.
>
> One option to make it clearer would be to add a link to:
>         https://www.kernel.org/category/releases.html
>
> And then use the terms:
>         - "Mainline release" for v6.10, v6.11, v6.12...
>         - "Prepatch release" for -rc1, -rc2, -rc3...
>
> What 2 and means are:
>
>         1. Features for the next mainline release:
>
>            - baseline shall be media.git ``next`` branch;
>
>         2. Fixes bugs at mainline releases:
         2. Bug fixes for the current mainline release

>
>            - baseline shall be the latest mainline release or media.git ``fixes``
>              if changes depend on a fix already merged;
>
>         3. Fixes for the next mainline release:
         3. Bug fixes for the next mainline release
>
>            - baseline shall be a prepatch release or media.git ``fixes``
            - baseline shall be a prepatch release (rcX) or media.git ``fixes``
>              if changes depend on a fix already merged;
>
>         .. Note:
>
>            See https://www.kernel.org/category/releases.html for an overview
>            about Kernel release types.
>
> Would the above text work best you? If not please propose another
> text.
>
> In practice, we'll likely accept other baselines for fixes as
> well, for practical reasons. From my side, I don't have any issues
> if I need to rebase a tested bugfix patch produced against an
> older Kernel send by an occasional contributor, if it only has
> trivial merge conflicts.
>
> > > +Patches with fixes shall have:
> > > +
> > > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > > +- when applicable, a ``Cc: stable@vger.kernel.org``.
> > > +
> > > +Patches that were fixing bugs publicly reported by someone at the
> > > +linux-media@vger.kernel.org mailing list shall have:
> > > +
> > > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > > +
> > > +Patches that change API shall update documentation accordingly at the
> > > +same patch series.
> > > +
> > > +See Documentation/process/index.rst for more details about e-mail submission.
> > > +
> > > +Once a patch is submitted, it may follow either one of the following
> > > +workflows:
> > > +
> > > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > > +
> > > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
> > > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > > +                +---------+   +-------+   +-----------------------+
> > > +
> > > +   For this workflow, pull requests can be generated by a committer,
> > > +   a previous committer, subsystem maintainers or by a trusted long-time
> > > +   contributor. If you are not in such group, please don't submit
> > > +   pull requests, as they will not be processed.
> > > +
> > > +b. Committers' workflow: patches are handled by media committers::
> > > +
> > > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > > +                +---------+   +--------------------+   +-----------+
> > > +
> > > +On both workflows, all patches shall be properly reviewed at
> > > +linux-media@vger.kernel.org before being merged at media-committers.git.
> > > +
> > > +When patches are picked by patchwork and when merged at media-committers,
> > > +CI bots will check for errors and may provide e-mail feedback about
> > > +patch problems. When this happens, the patch submitter must fix them, or
> > > +explain why the errors are false positives.
> > > +
> > > +Patches will only be moved to the next stage in those two workflows if they
> > > +don't fail on CI or if there are false-positives in the CI reports.
> > > +
> > > +Failures during e-mail submission
> > > ++++++++++++++++++++++++++++++++++
> > >
> > >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> > >  is submitted, the e-mail will first be accepted by the mailing list
> > > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> > >
> > >     - https://patchwork.linuxtv.org/project/linux-media/list/
> > >
> > > -If it doesn't automatically appear there after a few minutes, then
> > > +If it doesn't automatically appear there after some time [2]_, then
> > >  probably something went wrong on your submission. Please check if the
> > > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > > +email is in plain text\ [3]_ only and if your emailer is not mangling
> > >  whitespaces before complaining or submitting them again.
> > >
> > > -You can check if the mailing list server accepted your patch, by looking at:
> > > +To troubleshoot problems, you should first check if the mailing list
> > > +server has accepted your patch, by looking at:
> > >
> > >     - https://lore.kernel.org/linux-media/
> > >
> > > -.. [2] If your email contains HTML, the mailing list server will simply
> > > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > > +mangled the patch. Patchwork internally has a logic that checks if the
> >
> > a logic -> logic
> >
> > > +received e-mail contain a valid patch. Any whitespace and new line
> >
> > contain -> contains
> >
> > > +breakages mangling the patch won't be recognized by patchwork, thus such
> > > +patch will be rejected.
> > > +
> > > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > > +       the e-mail server may be busy, so it may take up to a few hours
> > > +       for a patch to be picked by patchwork.
> > > +
> > > +.. [3] If your email contains HTML, the mailing list server will simply
> > >         drop it, without any further notice.
> > >
> > > +.. _media-developers-gpg:
> > >
> > > -Media maintainers
> > > -+++++++++++++++++
> > > +Authentication for pull and merge requests
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >
> > > -At the media subsystem, we have a group of senior developers that
> > > -are responsible for doing the code reviews at the drivers (also known as
> > > -sub-maintainers), and another senior developer responsible for the
> > > -subsystem as a whole. For core changes, whenever possible, multiple
> > > -media maintainers do the review.
> > > +The authenticity of developers submitting pull requests and merge requests
> > > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
> >
> > sign -> signing
> >
> > Hmm, merge requests through gitlab are not signed. So this isn't correct, I
> > think.
>
> No, but they are authenticated based on gitlab's user ID. The authentication
> we'll have is when the new committer sends us an e-mail providing the
> gitlab's user ID.
>
> See patch 3, as it contains some changes aiming to better explain it. Once
> it get the same people reviewing that also reviewed 1 and 2, I'll fold it.
>
> >
> > >
> > > -The media maintainers that work on specific areas of the subsystem are:
> > > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> > >
> > > -- Remote Controllers (infrared):
> > > -    Sean Young <sean@mess.org>
> > > +For more details about PGP sign, please read
> > > +Documentation/process/maintainer-pgp-guide.rst.
> > >
> > > -- HDMI CEC:
> > > -    Hans Verkuil <hverkuil@xs4all.nl>
> > > +Subsystem maintainers
> > > +---------------------
> > >
> > > -- Media controller drivers:
> > > -    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > -
> > > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > > -    Sakari Ailus <sakari.ailus@linux.intel.com>
> > > -
> > > -- V4L2 drivers and core V4L2 frameworks:
> > > -    Hans Verkuil <hverkuil@xs4all.nl>
> > > -
> > > -The subsystem maintainer is:
> > > -  Mauro Carvalho Chehab <mchehab@kernel.org>
> > > -
> > > -Media maintainers may delegate a patch to other media maintainers as needed.
> > > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > > -responsible for reviewing a patch.
> > > +The subsystem maintainers are:
> > > +  - Mauro Carvalho Chehab <mchehab@kernel.org> and
> > > +  - Hans Verkuil <hverkuil@xs4all.nl>
> > >
> > >  Submit Checklist Addendum
> > >  -------------------------
> > > @@ -108,17 +225,14 @@ implementing the media APIs:
> > >  ====================       =======================================================
> > >  Type                       Tool
> > >  ====================       =======================================================
> > > -V4L2 drivers\ [3]_ ``v4l2-compliance``
> > > +V4L2 drivers\ [4]_ ``v4l2-compliance``
> > >  V4L2 virtual drivers       ``contrib/test/test-media``
> > >  CEC drivers                ``cec-compliance``
> > >  ====================       =======================================================
> > >
> > > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
> >
> > The ``v4l2-compliance`` utility
> >
> > (add 'utility')
>
> Ok.
>
> >
> > >         V4L2 drivers.
> > >
> > > -Other compilance tools are under development to check other parts of the
> > > -subsystem.
> > > -
> > >  Those tests need to pass before the patches go upstream.
> > >
> > >  Also, please notice that we build the Kernel with::
> > > @@ -134,6 +248,8 @@ Where the check script is::
> > >  Be sure to not introduce new warnings on your patches without a
> > >  very good reason.
> > >
> > > +Please see `Media development workflow`_ for e-mail submission rules.
> > > +
> > >  Style Cleanup Patches
> > >  +++++++++++++++++++++
> > >
> > > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> > >  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 in a couple of weeks or to ask
> > > -other developers to publicly add Reviewed-by and, more importantly,
> > > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> > >  ``Tested-by:`` tags.
> > >
> > >  Please note that we expect a detailed description for ``Tested-by:``,
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 1e930c7a58b1..c77f56a2e695 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> > >  M: Mauro Carvalho Chehab <mchehab@kernel.org>
> > >  L: linux-media@vger.kernel.org
> > >  S: Maintained
> > > +P: Documentation/driver-api/media/maintainer-entry-profile.rst
> > >  W: https://linuxtv.org
> > >  Q: http://patchwork.kernel.org/project/linux-media/list/
> >
> > Shouldn't this point to our patchwork instance instead?
>
> Good catch! I'll update it.
>
> >
> > >  T: git git://linuxtv.org/media.git
> >
> > Regards,
> >
> >       Hans
>
>
>
> Thanks,
> Mauro



-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 0/3] Document the new media-committer's model
  2024-12-02 15:03 ` [PATCH v3 0/3] Document the new media-committer's model Hans Verkuil
@ 2024-12-03  7:19   ` Mauro Carvalho Chehab
  2024-12-03 11:22     ` Laurent Pinchart
  0 siblings, 1 reply; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-03  7:19 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Jonathan Corbet, linux-doc, linux-kernel, workflows

Em Mon, 2 Dec 2024 16:03:45 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
> > The media subsystem used to have a multi-commiter's model in the
> > past, but things didn't go well on that time, and we had to move to
> > a centralized model.
> > 
> > As the community has evolved, and as there are now new policies in
> > place like CoC, let's experiment with a multi-committers again.
> > 
> > The model we're using was inspired by the DRM multi-committers
> > model. Yet, media subsystem is different on several aspects, so the
> > model is not exactly the same.
> > 
> > The implementation will be in phases. For this phase, the goal is that 
> > all committers will be people listed at MAINTAINERS.
> > 
> > On this series,:
> > 
> > patch 1: updates the  media maintainer's entry profile and adds the
> > workflow that will be used with the new model. While here, it also
> > adds a missing "P:" tag at the MAINTAINERS file, pointing to it;
> > 
> > patch 2: adds a new document focused at the new maintainers
> > process. Its target is for developers that will be granted with
> > commit rights at the new media-maintainers.git tree. It also
> > contains a reference tag addition to kernel.org PGP chain
> > at process/maintainer-pgp-guide.rst.
> > 
> > patch 3: make documents cleared about maintainership duties.  
> 
> At least from my perspective, v3 is close to being ready and I hope
> that v4 will be good enough to be merged.
> 
> That said, what is missing in all this is that there is nothing here
> that explains why you would want to become a media committer. It is all
> very dry stuff, lots of 'shall's, and 'rights' and 'trust' and obligations,
> but nothing about the satisfaction you get when you get the responsibility
> of a part of the kernel and being able to guide the development of that
> area.
> 
> It's good enough to get the multi-committer process off the ground, but
> it definitely needs more work to make it more inviting to become a media
> committer. Because right now it is as dry as dust.

Agreed. We focused on getting a document describing what it is expected
by committers, in order to start with the model. My view is that it works
fine for such purpose. I also feel that we're close to the final document.

I'm sending today a v4 addressing the comments since last review.

Once we get people that are already interested and ready to be on board,
and we know that the model and infrastructure works properly, we may implement
a phase 2 focusing on allowing more committers. For such purpose, we need to 
document the benefits/satisfaction of becoming a new committer. Depending how
it goes, either on phase 2 or on phase 3, we can change the model from 
invitation-only to volunteer-requests.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 2/3] docs: media: document media multi-committers rules and process
  2024-12-02 14:13   ` Sakari Ailus
@ 2024-12-03  8:26     ` Mauro Carvalho Chehab
  2024-12-03 10:03       ` Sakari Ailus
  0 siblings, 1 reply; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-03  8:26 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, Jonathan Corbet, linux-doc, linux-kernel, workflows,
	Hans Verkuil, Ricardo Ribalda

Em Mon, 2 Dec 2024 14:13:42 +0000
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> 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@kernel.org>
> > Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
> > Reviewed-by: Ricardo Ribalda <ribalda@chromium.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/
> 
> ?

Ok for all above.
> 
> > +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.

I'll add it at the note about linuxtv.org:

	These commit rights are granted with expectation of responsibility:
	committers are people who care about the Linux Kernel as a whole and
	about the Linux media subsystem and want to advance its development. It
	is also based on a trust relationship among other committers, maintainers
	and the Linux Media community[1].

	...


	[1] The Linux Media Community, also called LinuxTV Community, has its primary
	    site at https://linuxtv.org.
	
	    Media committers and developers are reachable via the #linux-media
	    IRC channel at OFTC.

> > +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/

Ok.

> 
> > +   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?

Yes. Tested on Sphinx 6.2.0.

> > +[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? :-)

Could you propose some text for that?

> > +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>`

No need. a Sphinx converts all *.rst into references automatically.

Better to use RST files at the text, as makes easier for people
reading the text file directly.

> > +
> > +  - 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?

Same as above.

> > +
> > +- 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/

Heh, I had to go to the dictionary to seek for "yearn" meaning ;-)

Let's use a simpler language, as most developers are not native-English
speakers. I did:

	s/intend/desire/

which is a synonym.

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

Ok.

> > +
> > +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/

Ok.

> > +
> > +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@linuxtv.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/ ?

Ok.

> 
> > +      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
> >  --------------------------------------------
> >    
> 



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties
  2024-12-02 14:48   ` Sakari Ailus
@ 2024-12-03  9:28     ` Mauro Carvalho Chehab
  2024-12-03 12:49       ` Sakari Ailus
  0 siblings, 1 reply; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-03  9:28 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, linux-kernel

Em Mon, 2 Dec 2024 14:48:53 +0000
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> Hi Mauro,
> 
> On Mon, Dec 02, 2024 at 10:26:21AM +0100, Mauro Carvalho Chehab wrote:
> > During the review of the media committes profile, it was noticed  
> 
> s/committe\K/r/

Addressed this and the other editorial changes.

> Can we expect people listed as maintainers to either send PRs or be media
> committers? I think this might be eventually the result but I think we're
> quite far from this currently and I expect things to remain that way in the
> near future.

Yes, having driver maintainers being committers and sending PRs is what we
expect to happen first.

For mid/long-term, once driver maintainers get in board, we may also have
other committers for the drivers whose maintainer is also a committer.

Now, having committers for drivers whose maintainer is not a committer
doesn't sound a good idea, except if such committer is doing just
janitorial work and gets A-B/R-B from the driver maintainer on all
patches he merges.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 2/3] docs: media: document media multi-committers rules and process
  2024-12-03  8:26     ` Mauro Carvalho Chehab
@ 2024-12-03 10:03       ` Sakari Ailus
  0 siblings, 0 replies; 26+ messages in thread
From: Sakari Ailus @ 2024-12-03 10:03 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, Jonathan Corbet, linux-doc, linux-kernel, workflows,
	Hans Verkuil, Ricardo Ribalda

Hi Mauro,

On Tue, Dec 03, 2024 at 09:26:13AM +0100, Mauro Carvalho Chehab wrote:
> > > +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.
> 
> I'll add it at the note about linuxtv.org:
> 
> 	These commit rights are granted with expectation of responsibility:
> 	committers are people who care about the Linux Kernel as a whole and
> 	about the Linux media subsystem and want to advance its development. It
> 	is also based on a trust relationship among other committers, maintainers
> 	and the Linux Media community[1].
> 
> 	...
> 
> 
> 	[1] The Linux Media Community, also called LinuxTV Community, has its primary
> 	    site at https://linuxtv.org.
> 	
> 	    Media committers and developers are reachable via the #linux-media
> 	    IRC channel at OFTC.

Looks good, thanks!

> 
> > > +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/
> 
> Ok.
> 
> > 
> > > +   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?
> 
> Yes. Tested on Sphinx 6.2.0.
> 
> > > +[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? :-)
> 
> Could you propose some text for that?

How about:

The most important aspects of volunteering to be a committer are that you
have demonstrated the ability to give good code reviews, interacting with
others in the community as well as common sense. These are what we're
looking for when we're judging whether you'd be a good Media committer.

> 
> > > +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>`
> 
> No need. a Sphinx converts all *.rst into references automatically.
> 
> Better to use RST files at the text, as makes easier for people
> reading the text file directly.

Ack.

> 
> > > +
> > > +  - 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?
> 
> Same as above.
> 
> > > +
> > > +- 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/
> 
> Heh, I had to go to the dictionary to seek for "yearn" meaning ;-)
> 
> Let's use a simpler language, as most developers are not native-English
> speakers. I did:
> 
> 	s/intend/desire/
> 
> which is a synonym.

Works for me.

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 0/3] Document the new media-committer's model
  2024-12-03  7:19   ` Mauro Carvalho Chehab
@ 2024-12-03 11:22     ` Laurent Pinchart
  2024-12-03 13:07       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2024-12-03 11:22 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hans Verkuil, linux-media, Jonathan Corbet, linux-doc,
	linux-kernel, workflows

On Tue, Dec 03, 2024 at 08:19:58AM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 2 Dec 2024 16:03:45 +0100 Hans Verkuil escreveu:
> > On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
> > > The media subsystem used to have a multi-commiter's model in the
> > > past, but things didn't go well on that time, and we had to move to
> > > a centralized model.
> > > 
> > > As the community has evolved, and as there are now new policies in
> > > place like CoC, let's experiment with a multi-committers again.
> > > 
> > > The model we're using was inspired by the DRM multi-committers
> > > model. Yet, media subsystem is different on several aspects, so the
> > > model is not exactly the same.
> > > 
> > > The implementation will be in phases. For this phase, the goal is that 
> > > all committers will be people listed at MAINTAINERS.
> > > 
> > > On this series,:
> > > 
> > > patch 1: updates the  media maintainer's entry profile and adds the
> > > workflow that will be used with the new model. While here, it also
> > > adds a missing "P:" tag at the MAINTAINERS file, pointing to it;
> > > 
> > > patch 2: adds a new document focused at the new maintainers
> > > process. Its target is for developers that will be granted with
> > > commit rights at the new media-maintainers.git tree. It also
> > > contains a reference tag addition to kernel.org PGP chain
> > > at process/maintainer-pgp-guide.rst.
> > > 
> > > patch 3: make documents cleared about maintainership duties.  
> > 
> > At least from my perspective, v3 is close to being ready and I hope
> > that v4 will be good enough to be merged.
> > 
> > That said, what is missing in all this is that there is nothing here
> > that explains why you would want to become a media committer. It is all
> > very dry stuff, lots of 'shall's, and 'rights' and 'trust' and obligations,
> > but nothing about the satisfaction you get when you get the responsibility
> > of a part of the kernel and being able to guide the development of that
> > area.
> > 
> > It's good enough to get the multi-committer process off the ground, but
> > it definitely needs more work to make it more inviting to become a media
> > committer. Because right now it is as dry as dust.
> 
> Agreed. We focused on getting a document describing what it is expected
> by committers, in order to start with the model. My view is that it works
> fine for such purpose. I also feel that we're close to the final document.
> 
> I'm sending today a v4 addressing the comments since last review.
> 
> Once we get people that are already interested and ready to be on board,
> and we know that the model and infrastructure works properly, we may implement
> a phase 2 focusing on allowing more committers. For such purpose, we need to 
> document the benefits/satisfaction of becoming a new committer. Depending how
> it goes, either on phase 2 or on phase 3, we can change the model from 
> invitation-only to volunteer-requests.

What's phase 3 ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers
  2024-12-02 13:23     ` Mauro Carvalho Chehab
@ 2024-12-03 11:52       ` Sakari Ailus
  0 siblings, 0 replies; 26+ messages in thread
From: Sakari Ailus @ 2024-12-03 11:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Hans Verkuil, Ricardo Ribalda

Hi Mauro,

On Mon, Dec 02, 2024 at 02:23:34PM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 2 Dec 2024 12:40:44 +0000
> Sakari Ailus <sakari.ailus@iki.fi> escreveu:
> 
> > Hi Mauro,
> > 
> > Thanks for the set.
> > 
> > Looks good overall, please still see my comments below.
> > 
> > On Mon, Dec 02, 2024 at 10:26:19AM +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.
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > Signed-off-by: Hans Verkuil <hverkuil@xs4ll.nl>
> > > Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> > >  MAINTAINERS                                   |   1 +
> > >  2 files changed, 163 insertions(+), 46 deletions(-)
> > > 
> > > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > index ffc712a5f632..dc764163cf1c 100644
> > > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> > >  Both media userspace and Kernel APIs are documented and the documentation
> > >  must be kept in sync with the API changes. It means that all patches that
> > >  add new features to the subsystem must also bring changes to the
> > > -corresponding API files.
> > > +corresponding API documentation files.
> > >  
> > > -Due to the size and wide scope of the media subsystem, media's
> > > -maintainership model is to have sub-maintainers that have a broad
> > > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > > -task to review the patches, providing feedback to users if the patches are
> > > +Due to the size and wide scope of the media subsystem, the media's
> > > +maintainership model is to have committers that have a broad knowledge of
> > > +a specific aspect of the subsystem. It is the committers' task to
> > > +review the patches, providing feedback to users if the patches are
> > >  following the subsystem rules and are properly using the media kernel and
> > >  userspace APIs.
> > >  
> > > -Patches for the media subsystem must be sent to the media mailing list
> > > -at linux-media@vger.kernel.org as plain text only e-mail. Emails with
> > > -HTML will be automatically rejected by the mail server. It could be wise
> > > -to also copy the sub-maintainer(s).
> > > +Media committers
> > > +----------------
> > > +
> > > +In the media subsystem, there are experienced developers who can push  
> > 
> > The media subsystem is generally understood to comprise of what's under
> > drivers/media, this should be referring to the community instead.
> 
> Do you have a proposal for a different text here?

How about:

In the Linux Media community developers with formal status are classified as
follows.

> 
> > 
> > > +patches directly to the development tree. These developers are called
> > > +Media committers and are divided into the following categories:
> > > +
> > > +- Committers:
> > > +    contributors for one or more drivers within the media subsystem.
> > > +    They can push changes to the tree that do not affect the core or ABI.
> > > +
> > > +- Core committers:
> > > +    responsible for part of the media core. They are typically
> > > +    responsible for one or more drivers within the media subsystem, but, besides
> > > +    that, they can also merge patches that change the code common to multiple
> > > +    drivers, including the kernel internal API.
> > > +
> > > +- Subsystem maintainers:  
> > 
> > s/Subsystem/Media tree/
> 
> Here, we're talking specifically about my hole and Hans, which will
> co-maintain the subsystem with me.. I guess subsystem maintainer is the 
> best to describe it.
> 
> Besides that, we used "media maintainers" in the past with a different
> meaning. Better to not re-use it here.

Seems good, although...

> > 
> > ?
> > > +    responsible for the subsystem as a whole, with access to the

I'd do here:

s/subsystem/Media tree/ or
s/subsystem/Media subsystem/

> > > +    entire subsystem.
> > > +
> > > +    Only subsystem maintainers can push changes that affect the userspace
> > > +    API/ABI.  
> > 
> > This is ambiguous. I think it should intend to say API/ABI changes require
> > approval from Media tree maintainers.
> 
> At the first moment, the idea is to commit them via PRs. So, no such
> commits will be merged by committers/core committers, but yeah, it
> could also be merged directly by a committer if it has our approval.
> 
> In any case, such changes need a consensus from the subsystem maintainers,
> which can just be based on a trust relationship between them with
> regards to certain parts of the subsystem, or via explicit acks.
> 
> Maybe:
> 
> 	API/ABI changes are done via consensus between subsystem
> 	maintainers.
> 
> 	Only subsystem maintainers push changes that affect the userspace
> 	API/ABI. Committers may push directly if they have approvals
> 	from subsystem maintainers.

Seems good to me. These could be in the same paragraph.

> 
> > What constitutes a UAPI change is a topic of discussion on its own. 
> 
> Everything that would break backward compatibility with existing non-kernel
> code are API/ABI changes.
> 
> > Does it
> > require adding a new IOCTL? Taking into use a reserved field? Changing
> > little-used driver behaviour slightly? 
> 
> For all of those, yes: any changes affecting the behavior or fields/nodes
> exported via ioctls and sysfs, including new V4L2 controls are API/ABI changes.
> 
> Some changes at OF are also API/ABI changes, but those can flow via
> committers, provided that OF maintainers added their review or acked-by.
> 
> > Fixing a bug in a driver?
> 
> No, fixing a bug, even if related to ABI/API non-compliance aren't.
> 
> Yet, ABI/API behavior changes at drivers shall not cause regressions.
> 
> I don't think we need to let it clear at the text

Works for me.

> 
> > The first two obviously yes, but the latter two probably not.
> > 
> > Also:
> > 
> > s/Only subsystem maintainers can push/Media tree maintainers' ack is
> > required for/
> > 
> > ?
> > 
> > > +
> > > +All media committers shall explicitly agree with the Kernel development process
> > > +as described at Documentation/process/index.rst and to the Kernel
> > > +development rules inside the Kernel documentation, including its code of
> > > +conduct.
> > > +
> > > +Media development tree
> > > +----------------------
> > > +
> > > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > > +instance where we track patches though their lifetime.
> > > +
> > > +The main tree used by media developers is at:
> > > +
> > > +https://git.linuxtv.org/media.git/
> > > +
> > > +.. _Media development workflow:
> > > +
> > > +Media development workflow
> > > +++++++++++++++++++++++++++
> > > +
> > > +All changes for the media subsystem must be sent first as e-mails to the
> > > +media mailing list, following the process documented at  
> > 
> > s/^/linux-/
> > 
> > Also I'd refer to it as "LMML".
> 
> We can add an alias there, but better to be explicit about what mailing
> list we're referring to.

The list is known as the Linux Media mailing list. We should refer it by
that name or an abbreviation (LMML).

> 
> > 
> > > +Documentation/process/index.rst.
> > > +
> > > +It means that patches shall be submitted as plain text only via e-mail to
> > > +linux-media@vger.kernel.org. While subscription is not mandatory, you
> > > +can find details about how to subscribe to it and to see its archives at:
> > > +
> > > +  https://subspace.kernel.org/vger.kernel.org.html
> > > +
> > > +Emails with HTML will be automatically rejected by the mail server.
> > > +
> > > +It could be wise to also copy the media committer(s). You should use
> > > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > > +Please always copy driver's authors and maintainers.
> > > +
> > > +Such patches need to be based against a public branch or tag as follows:
> > > +
> > > +1. Patches for new features need to be based at the ``next`` branch of
> > > +   media.git tree;
> > > +
> > > +2. Fixes against an already released kernel should preferably be against
> > > +   the latest released Kernel. If they require a previously-applied
> > > +   change at media.git tree, they need to be against its ``fixes`` branch.  
> > 
> > This would be a change to the current process that I don't think has been
> > discussed. If I understand correctly, generally this applies to patches
> > that have been merged to the development branch (formerly media stage tree
> > master) with Cc: stable and Fixes: tags.
> 
> 
> Not really a change: this is what we do in practice (except that we renamed
> master to next).

I mean fixes in particular. Fixes are generally meant for an *upcoming*
release, not a past release, and primarily fixing issues that have been
introduced in that cycle. Other bugfixes should go through the normal
process.

How about adding:

Patches with security implications should be handled using a different
process defined in Documentation/process/security-bugs.rst.

> 
> We did discuss that during the LPC week. There was a request there about
> simplifying the trees during media summit with some suggestions. I did a 
> followup meeting with Hans afterwards for us to check what would work best.
> 
> The change is, basically:
> 	media-tree master -> media.git next
> 	media-tree fixes   -> media.git fixes
> 
> 	media-stage master -> media-committers next
> 
> Subsystem maintainers are also merging patches at media-committers fixes,
> in order to let media-ci to test the fixes branch.
> 
> As agreed during the media summit, only subsystem maintainers will be
> merging fixes patches.
> 
> > > +
> > > +3. Fixes for issues not present at the latest released kernel shall
> > > +   be either against a -rc kernel for an upcoming release or
> > > +   against the ``fixes`` branch of the media.git tree.
> > > +
> > > +Patches with fixes shall have:
> > > +
> > > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > > +- when applicable, a ``Cc: stable@vger.kernel.org``.
> > > +
> > > +Patches that were fixing bugs publicly reported by someone at the
> > > +linux-media@vger.kernel.org mailing list shall have:
> > > +
> > > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > > +
> > > +Patches that change API shall update documentation accordingly at the
> > > +same patch series.
> > > +
> > > +See Documentation/process/index.rst for more details about e-mail submission.
> > > +
> > > +Once a patch is submitted, it may follow either one of the following
> > > +workflows:
> > > +
> > > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > > +
> > > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|  
> > 
> > s/e-mail/LMML/
> 
> maybe, instead: e-mail to LMML.

Sounds good.

> 
> > (see earlier comment)? Same below.
> > 
> > > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > > +                +---------+   +-------+   +-----------------------+
> > > +
> > > +   For this workflow, pull requests can be generated by a committer,
> > > +   a previous committer, subsystem maintainers or by a trusted long-time  
> > 
> > s/previous/former/
> > 
> > I'd also use plural in all cases here.
> 
> Ok.
> 
> > 
> > > +   contributor. If you are not in such group, please don't submit  
> > 
> > > +   pull requests, as they will not be processed.
> > > +
> > > +b. Committers' workflow: patches are handled by media committers::
> > > +
> > > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > > +                +---------+   +--------------------+   +-----------+
> > > +
> > > +On both workflows, all patches shall be properly reviewed at
> > > +linux-media@vger.kernel.org before being merged at media-committers.git.
> > > +
> > > +When patches are picked by patchwork and when merged at media-committers,
> > > +CI bots will check for errors and may provide e-mail feedback about
> > > +patch problems. When this happens, the patch submitter must fix them, or
> > > +explain why the errors are false positives.
> > > +
> > > +Patches will only be moved to the next stage in those two workflows if they
> > > +don't fail on CI or if there are false-positives in the CI reports.  
> > 
> > s/don't fail on/pass/
> 
> Ok.
> 
> > 
> > > +
> > > +Failures during e-mail submission
> > > ++++++++++++++++++++++++++++++++++
> > >  
> > >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> > >  is submitted, the e-mail will first be accepted by the mailing list
> > > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> > >  
> > >     - https://patchwork.linuxtv.org/project/linux-media/list/
> > >  
> > > -If it doesn't automatically appear there after a few minutes, then
> > > +If it doesn't automatically appear there after some time [2]_, then
> > >  probably something went wrong on your submission. Please check if the
> > > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > > +email is in plain text\ [3]_ only and if your emailer is not mangling
> > >  whitespaces before complaining or submitting them again.
> > >  
> > > -You can check if the mailing list server accepted your patch, by looking at:
> > > +To troubleshoot problems, you should first check if the mailing list
> > > +server has accepted your patch, by looking at:
> > >  
> > >     - https://lore.kernel.org/linux-media/
> > >  
> > > -.. [2] If your email contains HTML, the mailing list server will simply
> > > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > > +mangled the patch. Patchwork internally has a logic that checks if the
> > > +received e-mail contain a valid patch. Any whitespace and new line
> > > +breakages mangling the patch won't be recognized by patchwork, thus such
> > > +patch will be rejected.
> > > +
> > > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > > +       the e-mail server may be busy, so it may take up to a few hours
> > > +       for a patch to be picked by patchwork.
> > > +
> > > +.. [3] If your email contains HTML, the mailing list server will simply
> > >         drop it, without any further notice.
> > >  
> > > +.. _media-developers-gpg:
> > >  
> > > -Media maintainers
> > > -+++++++++++++++++
> > > +Authentication for pull and merge requests
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >  
> > > -At the media subsystem, we have a group of senior developers that
> > > -are responsible for doing the code reviews at the drivers (also known as
> > > -sub-maintainers), and another senior developer responsible for the
> > > -subsystem as a whole. For core changes, whenever possible, multiple
> > > -media maintainers do the review.
> > > +The authenticity of developers submitting pull requests and merge requests
> > > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
> > >  
> > > -The media maintainers that work on specific areas of the subsystem are:
> > > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> > >  
> > > -- Remote Controllers (infrared):
> > > -    Sean Young <sean@mess.org>
> > > +For more details about PGP sign, please read
> > > +Documentation/process/maintainer-pgp-guide.rst.  
> > 
> > s/.*/:ref:`the PGP guide <pgpguide>`/
> 
> No need. A Sphinx plugin does that automatically.

Ack.

> 
> > 
> > >  
> > > -- HDMI CEC:
> > > -    Hans Verkuil <hverkuil@xs4all.nl>
> > > +Subsystem maintainers
> > > +---------------------
> > >  
> > > -- Media controller drivers:
> > > -    Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > -
> > > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > > -    Sakari Ailus <sakari.ailus@linux.intel.com>
> > > -
> > > -- V4L2 drivers and core V4L2 frameworks:
> > > -    Hans Verkuil <hverkuil@xs4all.nl>
> > > -
> > > -The subsystem maintainer is:
> > > -  Mauro Carvalho Chehab <mchehab@kernel.org>
> > > -
> > > -Media maintainers may delegate a patch to other media maintainers as needed.
> > > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > > -responsible for reviewing a patch.
> > > +The subsystem maintainers are:
> > > +  - Mauro Carvalho Chehab <mchehab@kernel.org> and
> > > +  - Hans Verkuil <hverkuil@xs4all.nl>
> > >  
> > >  Submit Checklist Addendum
> > >  -------------------------
> > > @@ -108,17 +225,14 @@ implementing the media APIs:
> > >  ====================	=======================================================
> > >  Type			Tool
> > >  ====================	=======================================================
> > > -V4L2 drivers\ [3]_	``v4l2-compliance``
> > > +V4L2 drivers\ [4]_	``v4l2-compliance``
> > >  V4L2 virtual drivers	``contrib/test/test-media``
> > >  CEC drivers		``cec-compliance``
> > >  ====================	=======================================================
> > >  
> > > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
> > >         V4L2 drivers.
> > >  
> > > -Other compilance tools are under development to check other parts of the
> > > -subsystem.
> > > -
> > >  Those tests need to pass before the patches go upstream.
> > >  
> > >  Also, please notice that we build the Kernel with::
> > > @@ -134,6 +248,8 @@ Where the check script is::
> > >  Be sure to not introduce new warnings on your patches without a
> > >  very good reason.
> > >  
> > > +Please see `Media development workflow`_ for e-mail submission rules.
> > > +
> > >  Style Cleanup Patches
> > >  +++++++++++++++++++++
> > >  
> > > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> > >  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 in a couple of weeks or to ask
> > > -other developers to publicly add Reviewed-by and, more importantly,
> > > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> > >  ``Tested-by:`` tags.
> > >  
> > >  Please note that we expect a detailed description for ``Tested-by:``,
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 1e930c7a58b1..c77f56a2e695 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> > >  M:	Mauro Carvalho Chehab <mchehab@kernel.org>
> > >  L:	linux-media@vger.kernel.org
> > >  S:	Maintained
> > > +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
> > >  W:	https://linuxtv.org
> > >  Q:	http://patchwork.kernel.org/project/linux-media/list/
> > >  T:	git git://linuxtv.org/media.git  
> > 
> 
> 

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties
  2024-12-03  9:28     ` Mauro Carvalho Chehab
@ 2024-12-03 12:49       ` Sakari Ailus
  0 siblings, 0 replies; 26+ messages in thread
From: Sakari Ailus @ 2024-12-03 12:49 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, linux-kernel

Hi Mauro,

On Tue, Dec 03, 2024 at 10:28:46AM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 2 Dec 2024 14:48:53 +0000
> Sakari Ailus <sakari.ailus@iki.fi> escreveu:
> 
> > Hi Mauro,
> > 
> > On Mon, Dec 02, 2024 at 10:26:21AM +0100, Mauro Carvalho Chehab wrote:
> > > During the review of the media committes profile, it was noticed  
> > 
> > s/committe\K/r/
> 
> Addressed this and the other editorial changes.
> 
> > Can we expect people listed as maintainers to either send PRs or be media
> > committers? I think this might be eventually the result but I think we're
> > quite far from this currently and I expect things to remain that way in the
> > near future.
> 
> Yes, having driver maintainers being committers and sending PRs is what we
> expect to happen first.
> 
> For mid/long-term, once driver maintainers get in board, we may also have
> other committers for the drivers whose maintainer is also a committer.
> 
> Now, having committers for drivers whose maintainer is not a committer
> doesn't sound a good idea, except if such committer is doing just
> janitorial work and gets A-B/R-B from the driver maintainer on all
> patches he merges.

I meant primarily people listed in MAINTAINERS but who are at least not
yet Media committers.

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 0/3] Document the new media-committer's model
  2024-12-03 11:22     ` Laurent Pinchart
@ 2024-12-03 13:07       ` Mauro Carvalho Chehab
  2024-12-09  8:15         ` Media committers model postponed to 6.14 - Was: " Mauro Carvalho Chehab
  0 siblings, 1 reply; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-03 13:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hans Verkuil, linux-media, Jonathan Corbet, linux-doc,
	linux-kernel, workflows

Em Tue, 3 Dec 2024 13:22:09 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> On Tue, Dec 03, 2024 at 08:19:58AM +0100, Mauro Carvalho Chehab wrote:
> > Em Mon, 2 Dec 2024 16:03:45 +0100 Hans Verkuil escreveu:  
> > > On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:  
> > > > The media subsystem used to have a multi-commiter's model in the
> > > > past, but things didn't go well on that time, and we had to move to
> > > > a centralized model.
> > > > 
> > > > As the community has evolved, and as there are now new policies in
> > > > place like CoC, let's experiment with a multi-committers again.
> > > > 
> > > > The model we're using was inspired by the DRM multi-committers
> > > > model. Yet, media subsystem is different on several aspects, so the
> > > > model is not exactly the same.
> > > > 
> > > > The implementation will be in phases. For this phase, the goal is that 
> > > > all committers will be people listed at MAINTAINERS.
> > > > 
> > > > On this series,:
> > > > 
> > > > patch 1: updates the  media maintainer's entry profile and adds the
> > > > workflow that will be used with the new model. While here, it also
> > > > adds a missing "P:" tag at the MAINTAINERS file, pointing to it;
> > > > 
> > > > patch 2: adds a new document focused at the new maintainers
> > > > process. Its target is for developers that will be granted with
> > > > commit rights at the new media-maintainers.git tree. It also
> > > > contains a reference tag addition to kernel.org PGP chain
> > > > at process/maintainer-pgp-guide.rst.
> > > > 
> > > > patch 3: make documents cleared about maintainership duties.    
> > > 
> > > At least from my perspective, v3 is close to being ready and I hope
> > > that v4 will be good enough to be merged.
> > > 
> > > That said, what is missing in all this is that there is nothing here
> > > that explains why you would want to become a media committer. It is all
> > > very dry stuff, lots of 'shall's, and 'rights' and 'trust' and obligations,
> > > but nothing about the satisfaction you get when you get the responsibility
> > > of a part of the kernel and being able to guide the development of that
> > > area.
> > > 
> > > It's good enough to get the multi-committer process off the ground, but
> > > it definitely needs more work to make it more inviting to become a media
> > > committer. Because right now it is as dry as dust.  
> > 
> > Agreed. We focused on getting a document describing what it is expected
> > by committers, in order to start with the model. My view is that it works
> > fine for such purpose. I also feel that we're close to the final document.
> > 
> > I'm sending today a v4 addressing the comments since last review.
> > 
> > Once we get people that are already interested and ready to be on board,
> > and we know that the model and infrastructure works properly, we may implement
> > a phase 2 focusing on allowing more committers. For such purpose, we need to 
> > document the benefits/satisfaction of becoming a new committer. Depending how
> > it goes, either on phase 2 or on phase 3, we can change the model from 
> > invitation-only to volunteer-requests.  
> 
> What's phase 3 ?

The idea is to gradually open media-committers to more people, as each
phase succeeds, addressing infra, procedures, etc.

My rough idea is to do:

- Phase 0.99: beta testers;
- Phase 1 is to invite people that regularly submit PRs;
- Phase 2 is to invite other active maintainers;
- Phase 3 (or 2?, TBD) to open for non-maintainers.

We shouldn't rush it, as there are a lot to be done before opening it
broadly. So, I would say that:
- phase 0.99 would start in -rc2 (if things go well during this week); 
- phase 1 may still happen on this merge window, but as there will be
  only a few weeks between -rc2 and -rc6, and people usually get
  holidays in Dec/Jan, it is more likely that it will start for
  6.14-rc1, again if we didn't notice big issues on phase 0.99.

  We should wait at least for a couple of releases on phase 1,
  again to cleanup process and fine-tune infra. If things go well, 
  we can move to phase 2.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Media committers model postponed to 6.14 - Was: Re: [PATCH v3 0/3] Document the new media-committer's model
  2024-12-03 13:07       ` Mauro Carvalho Chehab
@ 2024-12-09  8:15         ` Mauro Carvalho Chehab
  2025-02-07 11:54           ` Hans Verkuil
  0 siblings, 1 reply; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2024-12-09  8:15 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hans Verkuil, linux-media, Jonathan Corbet, linux-doc,
	linux-kernel, workflows

Em Tue, 3 Dec 2024 14:07:12 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> 
> The idea is to gradually open media-committers to more people, as each
> phase succeeds, addressing infra, procedures, etc.
> 
> My rough idea is to do:
> 
> - Phase 0.99: beta testers;
> - Phase 1 is to invite people that regularly submit PRs;
> - Phase 2 is to invite other active maintainers;
> - Phase 3 (or 2?, TBD) to open for non-maintainers.
> 
> We shouldn't rush it, as there are a lot to be done before opening it
> broadly. So, I would say that:
> - phase 0.99 would start in -rc2 (if things go well during this week); 
> - phase 1 may still happen on this merge window, but as there will be
>   only a few weeks between -rc2 and -rc6, and people usually get
>   holidays in Dec/Jan, it is more likely that it will start for
>   6.14-rc1, again if we didn't notice big issues on phase 0.99.
> 
>   We should wait at least for a couple of releases on phase 1,
>   again to cleanup process and fine-tune infra. If things go well, 
>   we can move to phase 2.

After some discussions with Hans, we decided to postpone the
beta testers phase to the next kernel cycle. There are a couple of
reasons for that:

- This should give us more time to come up with a final version of 
  the media-committers documentation and agreement;

- This would also work better with regards to end of year's vacations,
  as they'll be affecting at least 2/3 -rc versions. Plus, we all have
  things to finish before such vacations. So, better to start fresh next
  year;

- Media CI still had issues with a patch series I submitted, as it picked
  the wrong baseline, causing CI to not test two patches that were
  applied on the top of media-committers/next branch. This was fixed
  by Ricardo, but it means that we may still need to polish CI before
  granting more people righs there.

With that, if we want to start the media committers for 6.14, we should
aim to close review this document by -rc6, or, at most, -rc7, getting 
the patches merged during the next merge window.

Regard

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Media committers model postponed to 6.14 - Was: Re: [PATCH v3 0/3] Document the new media-committer's model
  2024-12-09  8:15         ` Media committers model postponed to 6.14 - Was: " Mauro Carvalho Chehab
@ 2025-02-07 11:54           ` Hans Verkuil
  2025-02-10  7:00             ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 26+ messages in thread
From: Hans Verkuil @ 2025-02-07 11:54 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Laurent Pinchart
  Cc: linux-media, Jonathan Corbet, linux-doc, linux-kernel, workflows

On 09/12/2024 09:15, Mauro Carvalho Chehab wrote:
> Em Tue, 3 Dec 2024 14:07:12 +0100
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> 
>>
>> The idea is to gradually open media-committers to more people, as each
>> phase succeeds, addressing infra, procedures, etc.
>>
>> My rough idea is to do:
>>
>> - Phase 0.99: beta testers;
>> - Phase 1 is to invite people that regularly submit PRs;
>> - Phase 2 is to invite other active maintainers;
>> - Phase 3 (or 2?, TBD) to open for non-maintainers.
>>
>> We shouldn't rush it, as there are a lot to be done before opening it
>> broadly. So, I would say that:
>> - phase 0.99 would start in -rc2 (if things go well during this week); 
>> - phase 1 may still happen on this merge window, but as there will be
>>   only a few weeks between -rc2 and -rc6, and people usually get
>>   holidays in Dec/Jan, it is more likely that it will start for
>>   6.14-rc1, again if we didn't notice big issues on phase 0.99.
>>
>>   We should wait at least for a couple of releases on phase 1,
>>   again to cleanup process and fine-tune infra. If things go well, 
>>   we can move to phase 2.
> 
> After some discussions with Hans, we decided to postpone the
> beta testers phase to the next kernel cycle. There are a couple of
> reasons for that:
> 
> - This should give us more time to come up with a final version of 
>   the media-committers documentation and agreement;

Where are we with this? I haven't seen any updates since this post.

Personally, I think the CI is ready for more committers, so it would be
nice if we can get some experience with that.

Regards,

	Hans

> 
> - This would also work better with regards to end of year's vacations,
>   as they'll be affecting at least 2/3 -rc versions. Plus, we all have
>   things to finish before such vacations. So, better to start fresh next
>   year;
> 
> - Media CI still had issues with a patch series I submitted, as it picked
>   the wrong baseline, causing CI to not test two patches that were
>   applied on the top of media-committers/next branch. This was fixed
>   by Ricardo, but it means that we may still need to polish CI before
>   granting more people righs there.
> 
> With that, if we want to start the media committers for 6.14, we should
> aim to close review this document by -rc6, or, at most, -rc7, getting 
> the patches merged during the next merge window.
> 
> Regard
> 
> Thanks,
> Mauro
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Media committers model postponed to 6.14 - Was: Re: [PATCH v3 0/3] Document the new media-committer's model
  2025-02-07 11:54           ` Hans Verkuil
@ 2025-02-10  7:00             ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 26+ messages in thread
From: Mauro Carvalho Chehab @ 2025-02-10  7:00 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Laurent Pinchart, linux-media, Jonathan Corbet, linux-doc,
	linux-kernel, workflows

Em Fri, 7 Feb 2025 12:54:52 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 09/12/2024 09:15, Mauro Carvalho Chehab wrote:
> > Em Tue, 3 Dec 2024 14:07:12 +0100
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> >   
> >>
> >> The idea is to gradually open media-committers to more people, as each
> >> phase succeeds, addressing infra, procedures, etc.
> >>
> >> My rough idea is to do:
> >>
> >> - Phase 0.99: beta testers;
> >> - Phase 1 is to invite people that regularly submit PRs;
> >> - Phase 2 is to invite other active maintainers;
> >> - Phase 3 (or 2?, TBD) to open for non-maintainers.
> >>
> >> We shouldn't rush it, as there are a lot to be done before opening it
> >> broadly. So, I would say that:
> >> - phase 0.99 would start in -rc2 (if things go well during this week); 
> >> - phase 1 may still happen on this merge window, but as there will be
> >>   only a few weeks between -rc2 and -rc6, and people usually get
> >>   holidays in Dec/Jan, it is more likely that it will start for
> >>   6.14-rc1, again if we didn't notice big issues on phase 0.99.
> >>
> >>   We should wait at least for a couple of releases on phase 1,
> >>   again to cleanup process and fine-tune infra. If things go well, 
> >>   we can move to phase 2.  
> > 
> > After some discussions with Hans, we decided to postpone the
> > beta testers phase to the next kernel cycle. There are a couple of
> > reasons for that:
> > 
> > - This should give us more time to come up with a final version of 
> >   the media-committers documentation and agreement;  
> 
> Where are we with this? I haven't seen any updates since this post.
> 
> Personally, I think the CI is ready for more committers, so it would be
> nice if we can get some experience with that.

IMO, there are still some pending issues.

We still need to reach a consensus about what media maintainers will do.
I have to confess that discussing last time was painful due to some
arguments going too personal to my taste. Anyway, discussing it during
the end of the year was not a good idea as people (including myself) were
busy completing their yearly tasks. Also, people were taking vacations.

At the end of the last year, I finally rewrote the scripts I use on my
workflow. I'd like to test them during this cycle and see how it
behaves. 

While doing that, I noticed that we really need to have something like
margebot to help our workflow. From my side, I'd like to have one separate
MR per each separate patch series, as this makes easier to document the
main changes that the media subsystem is merging. I hope to test them more
during this Kernel cycle to be sure that everything is in place.

While using my scripts, I ended having 4 or 5 pending MRs at
media-committers. As we want a clean history without being bloated with
merges from temp trees/branches, we need to have some automation to help
basing the commits on the top of the branches.

The idea is to have margebot enabled there, meaning that committers
may delegate patches to margebot and let it automatically place patches
on the top of the branches. However, margebot has currently a problem:
it mangles with committer's metadata. Ricardo have been working upstream
and they are reaching a consensus about how to support preserving
committer data with margebot. IMO, we need that before having more
committers.

Finally, we need to have useful data to prepare changelog summary
upstream. In the past, as we were reviewing everything, it was easier
to identify the core changes (besides fixes/cleanups). With a
multicommitter's model, we need to rely on what committers will be
providing us. The idea I've been playing with, and that's what I
ended implementing on last submission(*), is to generate it based on
what each merge metadata contains.

(*) Yet, the level of information there were very inconsistent.
We need to do better during this cycle.

> Regards,
> 
> 	Hans
> 
> > 
> > - This would also work better with regards to end of year's vacations,
> >   as they'll be affecting at least 2/3 -rc versions. Plus, we all have
> >   things to finish before such vacations. So, better to start fresh next
> >   year;
> > 
> > - Media CI still had issues with a patch series I submitted, as it picked
> >   the wrong baseline, causing CI to not test two patches that were
> >   applied on the top of media-committers/next branch. This was fixed
> >   by Ricardo, but it means that we may still need to polish CI before
> >   granting more people righs there.
> > 
> > With that, if we want to start the media committers for 6.14, we should
> > aim to close review this document by -rc6, or, at most, -rc7, getting 
> > the patches merged during the next merge window.
> > 
> > Regard
> > 
> > Thanks,
> > Mauro
> >   
> 



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2025-02-10  7:00 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-02  9:26 [PATCH v3 0/3] Document the new media-committer's model Mauro Carvalho Chehab
2024-12-02  9:26 ` [PATCH v3 1/3] docs: media: update maintainer-entry-profile for multi-committers Mauro Carvalho Chehab
2024-12-02 12:40   ` Sakari Ailus
2024-12-02 13:23     ` Mauro Carvalho Chehab
2024-12-03 11:52       ` Sakari Ailus
2024-12-02 13:17   ` Hans Verkuil
2024-12-02 13:54     ` Mauro Carvalho Chehab
2024-12-02 14:49       ` Hans Verkuil
2024-12-02 20:48       ` Ricardo Ribalda
2024-12-02  9:26 ` [PATCH v3 2/3] docs: media: document media multi-committers rules and process Mauro Carvalho Chehab
2024-12-02 14:13   ` Sakari Ailus
2024-12-03  8:26     ` Mauro Carvalho Chehab
2024-12-03 10:03       ` Sakari Ailus
2024-12-02 14:37   ` Hans Verkuil
2024-12-02  9:26 ` [PATCH v3 3/3] docs: media: profile: make it clearer about maintainership duties Mauro Carvalho Chehab
2024-12-02 14:40   ` Hans Verkuil
2024-12-02 14:48   ` Sakari Ailus
2024-12-03  9:28     ` Mauro Carvalho Chehab
2024-12-03 12:49       ` Sakari Ailus
2024-12-02 15:03 ` [PATCH v3 0/3] Document the new media-committer's model Hans Verkuil
2024-12-03  7:19   ` Mauro Carvalho Chehab
2024-12-03 11:22     ` Laurent Pinchart
2024-12-03 13:07       ` Mauro Carvalho Chehab
2024-12-09  8:15         ` Media committers model postponed to 6.14 - Was: " Mauro Carvalho Chehab
2025-02-07 11:54           ` Hans Verkuil
2025-02-10  7:00             ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox