Message ID | cover.1733131405.git.mchehab+huawei@kernel.org |
---|---|
Headers | show |
Series | Document the new media-committer's model | expand |
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
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.
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