diff options
Diffstat (limited to 'Documentation/maintainer')
| -rw-r--r-- | Documentation/maintainer/configure-git.rst | 38 | ||||
| -rw-r--r-- | Documentation/maintainer/feature-and-driver-maintainers.rst | 166 | ||||
| -rw-r--r-- | Documentation/maintainer/index.rst | 3 | ||||
| -rw-r--r-- | Documentation/maintainer/maintainer-entry-profile.rst | 11 | ||||
| -rw-r--r-- | Documentation/maintainer/messy-diffstat.rst | 96 | ||||
| -rw-r--r-- | Documentation/maintainer/modifying-patches.rst | 50 | ||||
| -rw-r--r-- | Documentation/maintainer/pull-requests.rst | 8 | ||||
| -rw-r--r-- | Documentation/maintainer/rebasing-and-merging.rst | 6 |
8 files changed, 345 insertions, 33 deletions
diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst index 80ae5030a590..0a36831814ea 100644 --- a/Documentation/maintainer/configure-git.rst +++ b/Documentation/maintainer/configure-git.rst @@ -1,35 +1,31 @@ -.. _configuregit: - -Configure Git -============= +Configuring Git +=============== This chapter describes maintainer level git configuration. -Tagged branches used in :ref:`Documentation/maintainer/pull-requests.rst -<pullrequests>` should be signed with the developers public GPG key. Signed -tags can be created by passing the ``-u`` flag to ``git tag``. However, -since you would *usually* use the same key for the same project, you can -set it once with -:: +Tagged branches used in pull requests (see +Documentation/maintainer/pull-requests.rst) should be signed with the +developers public GPG key. Signed tags can be created by passing +``-u <key-id>`` to ``git tag``. However, since you would *usually* use the same +key for the project, you can set it in the configuration and use the ``-s`` +flag. To set the default ``key-id`` use:: git config user.signingkey "keyname" -Alternatively, edit your ``.git/config`` or ``~/.gitconfig`` file by hand: -:: +Alternatively, edit your ``.git/config`` or ``~/.gitconfig`` file by hand:: [user] name = Jane Developer email = jd@domain.org signingkey = jd@domain.org -You may need to tell ``git`` to use ``gpg2`` -:: +You may need to tell ``git`` to use ``gpg2``:: [gpg] program = /path/to/gpg2 -You may also like to tell ``gpg`` which ``tty`` to use (add to your shell rc file) -:: +You may also like to tell ``gpg`` which ``tty`` to use (add to your shell +rc file):: export GPG_TTY=$(tty) @@ -37,26 +33,24 @@ You may also like to tell ``gpg`` which ``tty`` to use (add to your shell rc fil Creating commit links to lore.kernel.org ---------------------------------------- -The web site http://lore.kernel.org is meant as a grand archive of all mail +The web site https://lore.kernel.org is meant as a grand archive of all mail list traffic concerning or influencing the kernel development. Storing archives of patches here is a recommended practice, and when a maintainer applies a patch to a subsystem tree, it is a good idea to provide a Link: tag with a reference back to the lore archive so that people that browse the commit history can find related discussions and rationale behind a certain change. -The link tag will look like this: +The link tag will look like this:: Link: https://lore.kernel.org/r/<message-id> This can be configured to happen automatically any time you issue ``git am`` -by adding the following hook into your git: - -.. code-block:: none +by adding the following hook into your git:: $ git config am.messageid true $ cat >.git/hooks/applypatch-msg <<'EOF' #!/bin/sh . git-sh-setup - perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1" + perl -pi -e 's|^Message-I[dD]:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1" test -x "$GIT_DIR/hooks/commit-msg" && exec "$GIT_DIR/hooks/commit-msg" ${1+"$@"} : diff --git a/Documentation/maintainer/feature-and-driver-maintainers.rst b/Documentation/maintainer/feature-and-driver-maintainers.rst new file mode 100644 index 000000000000..fb94a9b29061 --- /dev/null +++ b/Documentation/maintainer/feature-and-driver-maintainers.rst @@ -0,0 +1,166 @@ +.. SPDX-License-Identifier: GPL-2.0 + +============================== +Feature and driver maintainers +============================== + +The term "maintainer" spans a very wide range of levels of engagement +from people handling patches and pull requests as almost a full time job +to people responsible for a small feature or a driver. + +Unlike most of the chapter, this section is meant for the latter (more +populous) group. It provides tips and describes the expectations and +responsibilities of maintainers of a small(ish) section of the code. + +Drivers and alike most often do not have their own mailing lists and +git trees but instead send and review patches on the list of a larger +subsystem. + +Responsibilities +================ + +The amount of maintenance work is usually proportional to the size +and popularity of the code base. Small features and drivers should +require relatively small amount of care and feeding. Nonetheless +when the work does arrive (in form of patches which need review, +user bug reports etc.) it has to be acted upon promptly. +Even when a particular driver only sees one patch a month, or a quarter, +a subsystem could well have a hundred such drivers. Subsystem +maintainers cannot afford to wait a long time to hear from reviewers. + +The exact expectations on the response time will vary by subsystem. +The patch review SLA the subsystem had set for itself can sometimes +be found in the subsystem documentation. Failing that as a rule of thumb +reviewers should try to respond quicker than what is the usual patch +review delay of the subsystem maintainer. The resulting expectations +may range from two working days for fast-paced subsystems (e.g. networking) +to as long as a few weeks in slower moving parts of the kernel. + +Mailing list participation +-------------------------- + +Linux kernel uses mailing lists as the primary form of communication. +Maintainers must be subscribed and follow the appropriate subsystem-wide +mailing list. Either by subscribing to the whole list or using more +modern, selective setup like +`lei <https://people.kernel.org/monsieuricon/lore-lei-part-1-getting-started>`_. + +Maintainers must know how to communicate on the list (plain text, no invasive +legal footers, no top posting, etc.) + +Reviews +------- + +Maintainers must review *all* patches touching exclusively their drivers, +no matter how trivial. If the patch is a tree wide change and modifies +multiple drivers - whether to provide a review is left to the maintainer. + +When there are multiple maintainers for a piece of code an ``Acked-by`` +or ``Reviewed-by`` tag (or review comments) from a single maintainer is +enough to satisfy this requirement. + +If the review process or validation for a particular change will take longer +than the expected review timeline for the subsystem, maintainer should +reply to the submission indicating that the work is being done, and when +to expect full results. + +Refactoring and core changes +---------------------------- + +Occasionally core code needs to be changed to improve the maintainability +of the kernel as a whole. Maintainers are expected to be present and +help guide and test changes to their code to fit the new infrastructure. + +Bug reports +----------- + +Maintainers must ensure severe problems in their code reported to them +are resolved in a timely manner: regressions, kernel crashes, kernel warnings, +compilation errors, lockups, data loss, and other bugs of similar scope. + +Maintainers furthermore should respond to reports about other kinds of +bugs as well, if the report is of reasonable quality or indicates a +problem that might be severe -- especially if they have *Supported* +status of the codebase in the MAINTAINERS file. + +Open development +---------------- + +Discussions about user reported issues, and development of new code +should be conducted in a manner typical for the larger subsystem. +It is common for development within a single company to be conducted +behind closed doors. However, development and discussions initiated +by community members must not be redirected from public to closed forums +or to private email conversations. Reasonable exceptions to this guidance +include discussions about security related issues. + +Selecting the maintainer +======================== + +The previous section described the expectations of the maintainer, +this section provides guidance on selecting one and describes common +misconceptions. + +The author +---------- + +Most natural and common choice of a maintainer is the author of the code. +The author is intimately familiar with the code, so it is the best person +to take care of it on an ongoing basis. + +That said, being a maintainer is an active role. The MAINTAINERS file +is not a list of credits (in fact a separate CREDITS file exists), +it is a list of those who will actively help with the code. +If the author does not have the time, interest or ability to maintain +the code, a different maintainer must be selected. + +Multiple maintainers +-------------------- + +Modern best practices dictate that there should be at least two maintainers +for any piece of code, no matter how trivial. It spreads the burden, helps +people take vacations and prevents burnout, trains new members of +the community etc. etc. Even when there is clearly one perfect candidate, +another maintainer should be found. + +Maintainers must be human, therefore, it is not acceptable to add a mailing +list or a group email as a maintainer. Trust and understanding are the +foundation of kernel maintenance and one cannot build trust with a mailing +list. Having a mailing list *in addition* to humans is perfectly fine. + +Corporate structures +-------------------- + +To an outsider the Linux kernel may resemble a hierarchical organization +with Linus as the CEO. While the code flows in a hierarchical fashion, +the corporate template does not apply here. Linux is an anarchy held +together by (rarely expressed) mutual respect, trust and convenience. + +All that is to say that managers almost never make good maintainers. +The maintainer position more closely matches an on-call rotation +than a position of power. + +The following characteristics of a person selected as a maintainer +are clear red flags: + + - unknown to the community, never sent an email to the list before + - did not author any of the code + - (when development is contracted) works for a company which paid + for the development rather than the company which did the work + +Non compliance +============== + +Subsystem maintainers may remove inactive maintainers from the MAINTAINERS +file. If the maintainer was a significant author or played an important +role in the development of the code, they should be moved to the CREDITS file. + +Removing an inactive maintainer should not be seen as a punitive action. +Having an inactive maintainer has a real cost as all developers have +to remember to include the maintainers in discussions and subsystem +maintainers spend brain power figuring out how to solicit feedback. + +Subsystem maintainers may remove code for lacking maintenance. + +Subsystem maintainers may refuse accepting code from companies +which repeatedly neglected their maintainership duties. diff --git a/Documentation/maintainer/index.rst b/Documentation/maintainer/index.rst index d904e74e1159..eeee27f8b18c 100644 --- a/Documentation/maintainer/index.rst +++ b/Documentation/maintainer/index.rst @@ -9,8 +9,11 @@ additions to this manual. .. toctree:: :maxdepth: 2 + feature-and-driver-maintainers configure-git rebasing-and-merging pull-requests + messy-diffstat maintainer-entry-profile + modifying-patches diff --git a/Documentation/maintainer/maintainer-entry-profile.rst b/Documentation/maintainer/maintainer-entry-profile.rst index 77e43c8b24b4..cda5d691e967 100644 --- a/Documentation/maintainer/maintainer-entry-profile.rst +++ b/Documentation/maintainer/maintainer-entry-profile.rst @@ -31,7 +31,7 @@ Example questions to consider: - What branch should contributors submit against? - Links to any other Maintainer Entry Profiles? For example a device-driver may point to an entry for its parent subsystem. This makes - the contributor aware of obligations a maintainer may have have for + the contributor aware of obligations a maintainer may have for other maintainers in the submission chain. @@ -101,3 +101,12 @@ to do something different in the near future. ../doc-guide/maintainer-profile ../nvdimm/maintainer-entry-profile + ../arch/riscv/patch-acceptance + ../process/maintainer-soc + ../process/maintainer-soc-clean-dts + ../driver-api/media/maintainer-entry-profile + ../process/maintainer-netdev + ../driver-api/vfio-pci-device-specific-driver-acceptance + ../nvme/feature-and-quirk-policy + ../filesystems/xfs/xfs-maintainer-entry-profile + ../mm/damon/maintainer-profile diff --git a/Documentation/maintainer/messy-diffstat.rst b/Documentation/maintainer/messy-diffstat.rst new file mode 100644 index 000000000000..c015f66d7621 --- /dev/null +++ b/Documentation/maintainer/messy-diffstat.rst @@ -0,0 +1,96 @@ +.. SPDX-License-Identifier: GPL-2.0 + +===================================== +Handling messy pull-request diffstats +===================================== + +Subsystem maintainers routinely use ``git request-pull`` as part of the +process of sending work upstream. Normally, the result includes a nice +diffstat that shows which files will be touched and how much of each will +be changed. Occasionally, though, a repository with a relatively +complicated development history will yield a massive diffstat containing a +great deal of unrelated work. The result looks ugly and obscures what the +pull request is actually doing. This document describes what is happening +and how to fix things up; it is derived from The Wisdom of Linus Torvalds, +found in Linus1_ and Linus2_. + +.. _Linus1: https://lore.kernel.org/lkml/CAHk-=wg3wXH2JNxkQi+eLZkpuxqV+wPiHhw_Jf7ViH33Sw7PHA@mail.gmail.com/ +.. _Linus2: https://lore.kernel.org/lkml/CAHk-=wgXbSa8yq8Dht8at+gxb_idnJ7X5qWZQWRBN4_CUPr=eQ@mail.gmail.com/ + +A Git development history proceeds as a series of commits. In a simplified +manner, mainline kernel development looks like this:: + + ... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN + +If one wants to see what has changed between two points, a command like +this will do the job:: + + $ git diff --stat --summary vN-rc2..vN-rc3 + +Here, there are two clear points in the history; Git will essentially +"subtract" the beginning point from the end point and display the resulting +differences. The requested operation is unambiguous and easy enough to +understand. + +When a subsystem maintainer creates a branch and commits changes to it, the +result in the simplest case is a history that looks like:: + + ... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN + | + +-- c1 --- c2 --- ... --- cN + +If that maintainer now uses ``git diff`` to see what has changed between +the mainline branch (let's call it "linus") and cN, there are still two +clear endpoints, and the result is as expected. So a pull request +generated with ``git request-pull`` will also be as expected. But now +consider a slightly more complex development history:: + + ... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN + | | + | +-- c1 --- c2 --- ... --- cN + | / + +-- x1 --- x2 --- x3 + +Our maintainer has created one branch at vN-rc1 and another at vN-rc2; the +two were then subsequently merged into c2. Now a pull request generated +for cN may end up being messy indeed, and developers often end up wondering +why. + +What is happening here is that there are no longer two clear end points for +the ``git diff`` operation to use. The development culminating in cN +started in two different places; to generate the diffstat, ``git diff`` +ends up having pick one of them and hoping for the best. If the diffstat +starts at vN-rc1, it may end up including all of the changes between there +and the second origin end point (vN-rc2), which is certainly not what our +maintainer had in mind. With all of that extra junk in the diffstat, it +may be impossible to tell what actually happened in the changes leading up +to cN. + +Maintainers often try to resolve this problem by, for example, rebasing the +branch or performing another merge with the linus branch, then recreating +the pull request. This approach tends not to lead to joy at the receiving +end of that pull request; rebasing and/or merging just before pushing +upstream is a well-known way to get a grumpy response. + +So what is to be done? The best response when confronted with this +situation is to indeed to do a merge with the branch you intend your work +to be pulled into, but to do it privately, as if it were the source of +shame. Create a new, throwaway branch and do the merge there:: + + ... vM --- vN-rc1 --- vN-rc2 --- vN-rc3 --- ... --- vN-rc7 --- vN + | | | + | +-- c1 --- c2 --- ... --- cN | + | / | | + +-- x1 --- x2 --- x3 +------------+-- TEMP + +The merge operation resolves all of the complications resulting from the +multiple beginning points, yielding a coherent result that contains only +the differences from the mainline branch. Now it will be possible to +generate a diffstat with the desired information:: + + $ git diff -C --stat --summary linus..TEMP + +Save the output from this command, then simply delete the TEMP branch; +definitely do not expose it to the outside world. Take the saved diffstat +output and edit it into the messy pull request, yielding a result that +shows what is really going on. That request can then be sent upstream. diff --git a/Documentation/maintainer/modifying-patches.rst b/Documentation/maintainer/modifying-patches.rst new file mode 100644 index 000000000000..58385d2e8065 --- /dev/null +++ b/Documentation/maintainer/modifying-patches.rst @@ -0,0 +1,50 @@ +.. _modifyingpatches: + +Modifying Patches +================= + +If you are a subsystem or branch maintainer, sometimes you need to slightly +modify patches you receive in order to merge them, because the code is not +exactly the same in your tree and the submitters'. If you stick strictly to +rule (c) of the developers certificate of origin, you should ask the submitter +to rediff, but this is a totally counter-productive waste of time and energy. +Rule (b) allows you to adjust the code, but then it is very impolite to change +one submitters code and make him endorse your bugs. To solve this problem, it +is recommended that you add a line between the last Signed-off-by header and +yours, indicating the nature of your changes. While there is nothing mandatory +about this, it seems like prepending the description with your mail and/or +name, all enclosed in square brackets, is noticeable enough to make it obvious +that you are responsible for last-minute changes. Example:: + + Signed-off-by: Random J Developer <random@developer.example.org> + [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h] + Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org> + +This practice is particularly helpful if you maintain a stable branch and +want at the same time to credit the author, track changes, merge the fix, +and protect the submitter from complaints. Note that under no circumstances +can you change the author's identity (the From header), as it is the one +which appears in the changelog. + +Special note to back-porters: It seems to be a common and useful practice +to insert an indication of the origin of a patch at the top of the commit +message (just after the subject line) to facilitate tracking. For instance, +here's what we see in a 3.x-stable release:: + + Date: Tue Oct 7 07:26:38 2014 -0400 + + libata: Un-break ATA blacklist + + commit 1c40279960bcd7d52dbdf1d466b20d24b99176c8 upstream. + +And here's what might appear in an older kernel once a patch is backported:: + + Date: Tue May 13 22:12:27 2008 +0200 + + wireless, airo: waitbusy() won't delay + + [backport of 2.6 commit b7acbdfbd1f277c1eb23f344f899cfa4cd0bf36a] + +Whatever the format, this information provides a valuable help to people +tracking your trees, and to people trying to troubleshoot bugs in your +tree. diff --git a/Documentation/maintainer/pull-requests.rst b/Documentation/maintainer/pull-requests.rst index 1a2f99b67d25..0d63d9d7e347 100644 --- a/Documentation/maintainer/pull-requests.rst +++ b/Documentation/maintainer/pull-requests.rst @@ -1,5 +1,3 @@ -.. _pullrequests: - Creating Pull Requests ====================== @@ -15,7 +13,7 @@ please direct abuse to Tobin C. Harding <me@tobin.cc>. Original email thread:: - http://lkml.kernel.org/r/20171114110500.GA21175@kroah.com + https://lore.kernel.org/r/20171114110500.GA21175@kroah.com Create Branch @@ -41,7 +39,7 @@ named ``char-misc-next``, you would be using the following command:: that will create a signed tag called ``char-misc-4.15-rc1`` based on the last commit in the ``char-misc-next`` branch, and sign it with your gpg key -(see :ref:`Documentation/maintainer/configure-git.rst <configuregit>`). +(see Documentation/maintainer/configure-git.rst). Linus will only accept pull requests based on a signed tag. Other maintainers may differ. @@ -52,7 +50,7 @@ so outline what is contained here, why it should be merged, and what, if any, testing has been done. All of this information will end up in the tag itself, and then in the merge commit that the maintainer makes if/when they merge the pull request. So write it up well, as it will be in the kernel -tree for forever. +tree forever. As said by Linus:: diff --git a/Documentation/maintainer/rebasing-and-merging.rst b/Documentation/maintainer/rebasing-and-merging.rst index 09f988e7fa71..85800ce95ae5 100644 --- a/Documentation/maintainer/rebasing-and-merging.rst +++ b/Documentation/maintainer/rebasing-and-merging.rst @@ -213,11 +213,7 @@ point rather than some random spot. If your upstream-bound branch has emptied entirely into the mainline during the merge window, you can pull it forward with a command like:: - git merge v5.2-rc1^0 - -The "^0" will cause Git to do a fast-forward merge (which should be -possible in this situation), thus avoiding the addition of a spurious merge -commit. + git merge --ff-only v5.2-rc1 The guidelines laid out above are just that: guidelines. There will always be situations that call out for a different solution, and these guidelines |