[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] docs: document patch rules
On Wed, Feb 02, 2022 at 12:44:48PM +0100, Juergen Gross wrote: > Add a document to describe the rules for sending a proper patch. > > As it contains all the information already being present in > docs/process/tags.pandoc remove that file. > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx> > --- > docs/process/sending-patches.pandoc | 284 ++++++++++++++++++++++++++++ > docs/process/tags.pandoc | 55 ------ > 2 files changed, 284 insertions(+), 55 deletions(-) > create mode 100644 docs/process/sending-patches.pandoc > delete mode 100644 docs/process/tags.pandoc > > diff --git a/docs/process/sending-patches.pandoc > b/docs/process/sending-patches.pandoc > new file mode 100644 > index 0000000000..4cfc6e1a5b > --- /dev/null > +++ b/docs/process/sending-patches.pandoc > @@ -0,0 +1,284 @@ > +# How a proper patch should look like > + > +This is a brief description how a proper patch for the Xen project should > +look like. Examples and tooling tips are not part of this document, those > +can be found in the > +[Xen Wiki](https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches). > + > +## The patch subject > + > +The first line at the top of the patch should contain a short description of > +what the patch does, and hints as to what code it touches. This line is used > +as the **Subject** line of the mail when sending the patch. > + > +The hint which code is touched us usually in form of a relative path inside > +the Xen git repository, where obvious directories can be omitted or replaced > +by abbreviations, or it can be a single word describing the topic: > + > + <path>: <description> I would use <component> maybe instead of path, to explicitly note this is not usually a real path inside the repo. > + > +E.g.: > + > + xen/arm: increase memory banks number define value > + tools/libs/evtchn: Deduplicate xenevtchn_fd() Mostly a nit, but since this document is about style: I wouldn't recommend using a capital letter after ':' by default. The above line should instead be: tools/libs/evtchn: deduplicate xenevtchn_fd() > + MAINTAINERS: update my email address > + build: correct usage comments in Kbuild.include > + > +The description should give a rough hint *what* is done in the patch. > + > +The subject line should in general not exceed 80 characters. It must be > +followed by a blank line. > + > +## The commit message > + > +The commit message is free text describing *why* the patch is done and > +*how* the goal of the patch is achieved. A good commit message will describe > +the current situation, the desired goal, and the way this goal is being > +achieved. Parts of that can be omitted in obvious cases. > + > +In case additional changes are done in the patch (like e.g. cleanups), those > +should be mentioned. > + > +When referencing other patches (e.g. `patch xy introduced a bug ...`) those > +patches should be referenced via their commit id (at least 12 digits) and the > +patch subject: > + > + Commit 67d01cdb5518 ("x86: infrastructure to allow converting certain > + indirect calls to direct ones") introduced a bug ... > + > +The following ``git config`` settings can be used to add a pretty format for > +outputting the above style in the ``git log`` or ``git show`` commands: > + > + [core] > + abbrev = 12 > + [pretty] > + fixes = Fixes: %h (\"%s\") > + > +Lines in the commit message should not exceed 75 characters, except when > +copying error output directly into the commit message. > + > +## Tags > + > +Tags are entries in the form > + > + Tag: something > + > +In general tags are added in chronological order. So a `Reviewed-by:` tag > +should be added **after** the `Signed-off-by:` tag, as the review happened > +after the patch was written. > + > +Do not split a tag across multiple lines, tags are exempt from the > +"wrap at 75 columns" rule in order to simplify parsing scripts. > + > +### Taken-from: > + > +Xen has inherited some source files from other open source projects. In case > +a patch modifying such an inherited file is taken from that project (maybe in > +modified form), the `Taken-from:` tag specifies the source of the patch: > + > + Taken-from: <repository-URL> <commit-id> > + > +E.g.: > + > + Taken-from: > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f093b08c47b3 > + > +All tags **above** the `Taken-from:` tag are from the original patch (which > +should all be kept), while tags **after** `Taken-from:` are related to the > +normal Xen patch process as described here. > + > +### Fixes: > + > +If your patch fixes a bug in a specific commit, e.g. you found an issue using > +``git bisect``, please use the `Fixes:` tag with the first 12 characters of > +the commit id, and the one line summary. > + > + Fixes: <commit-id> ("<patch-subject>") > + > +E.g.: > + > + Fixes: 67d01cdb5518 ("x86: infrastructure to allow converting certain > indirect calls to direct ones") > + > +### Backport: > + > +A backport tag is an optional tag in the commit message to request a > +given commit to be backported to the released trees: > + > + Backport: <version> [# <comment>] So we already had a documented usage of '#' in tags, which I think should make it a better candidate for the R-b scope limiting. > + > +E.g.: > + > + Backport: 4.9+ > + > +It marks a commit for being a candidate for backports to all released > +trees from 4.9 onward. > + > +The backport requester is expected to specify which currently supported > +releases need the backport; but encouraged to specify a release as far > +back as possible which applies. If the requester doesn't know the oldest > +affected tree, they are encouraged to append a comment like the > +following: > + > + Backport: 4.9+ # maybe older > + > +Maintainers request the Backport tag to be added on commit. Contributors > +are welcome to mark their patches with the Backport tag when they deem > +appropriate. Maintainers will request for it to be removed when that is > +not the case. > + > +Please note that the Backport tag is a **request** for backport, which > +will still need to be evaluated by the maintainers. Maintainers might > +ask the requester to help with the backporting work if it is not > +trivial. > + > +### Reported-by: > + > +This optional tag can be used to give credit to someone reporting an issue. > +It is in the format: > + > + Reported-by: name <email@domain> > + > +E.g.: > + > + Reported-by: Jane Doe <jane.doe@xxxxxxxxxxx> > + > +As the email address will be made public via git, the reporter of an issue > +should be asked whether he/she is fine with being mentioned in the patch. > + > +### Suggested-by: > + > +This optional tag can be used to give credit to someone having suggested the > +solution the patch is implementing. It is in the format: > + > + Suggested-by: name <email@domain> > + > +E.g.: > + > + Suggested-by: Jane Doe <jane.doe@xxxxxxxxxxx> > + > +As the email address will be made public via git, the reporter of an issue > +should be asked whether he/she is fine with being mentioned in the patch. > + > +### Signed-off-by: > + > +This mandatory tag specifies the author(s) of a patch (for each author a > +separate `Signed-off-by:` tag is needed). It is in the format: > + > + Signed-off-by: name <email@domain> > + > +E.g.: > + > + Signed-off-by: Jane Doe <jane.doe@xxxxxxxxxxx> > + > +The author must be a natural person (not a team or just a company) and the > +`Signed-off-by:` tag must include the real name of the author (no pseudonym). > + > +By signing the patch with her/his name the author explicitly confirms to have > +made the contribution conforming to the `Developer's Certificate of Origin`: > + > + Developer's Certificate of Origin 1.1 > + > + By making a contribution to this project, I certify that: > + > + (a) The contribution was created in whole or in part by me and I > + have the right to submit it under the open source license > + indicated in the file; or > + > + (b) The contribution is based upon previous work that, to the best > + of my knowledge, is covered under an appropriate open source > + license and I have the right under that license to submit that > + work with modifications, whether created in whole or in part > + by me, under the same open source license (unless I am > + permitted to submit under a different license), as indicated > + in the file; or > + > + (c) The contribution was provided directly to me by some other > + person who certified (a), (b) or (c) and I have not modified > + it. > + > + (d) I understand and agree that this project and the contribution > + are public and that a record of the contribution (including all > + personal information I submit with it, including my sign-off) is > + maintained indefinitely and may be redistributed consistent with > + this project or the open source license(s) involved. > + > +### Reviewed-by: > + > +A `Reviewed-by:` tag can only be given by a reviewer of the patch. With > +responding to a sent patch adding the `Reviewed-by:` tag the reviewer > +(which can be anybody) confirms to have looked thoroughly at the patch and > +didn't find any issue (being it technical, legal or formal ones). If the > +review is covering only some parts of the patch, those parts can optionally > +be specified (multiple areas can be covered with multiple `Reviewed-by:` > +tags). It is in the format: > + > + Reviewed-by: name <email@domain> [# area] > + > +E.g.: > + > + Reviewed-by: Jane Doe <jane.doe@xxxxxxxxxxx> > + Reviewed-by: Jane Doe <jane.doe@xxxxxxxxxxx> # xen/x86 I think you should mention in the commit message that we are also adding the R-b scope limiting in this commit? The commit message makes it look like this is mostly moving the existing Tags into a new document. > + > +In case a patch is being resent an already given `Reviewed-by:` tag can and > +should be included, if the patch didn't change the portions of the patch > +covered by the tag, or if the reviewer already made clear it would be fine > +to make specific changes and no *other* changes have been made. > + > +### Acked-by: > + > +Similar to `Reviewed-by:` the `Acked-by:` tag is given by someone having > looked > +at the patch. The `Acked-by:` tag can only be given by a **maintainer** of > the > +modified code, and it only covers the code the maintainer is responsible for. > +For this reason there is no optional area possible. With the `Acked-by:` tag > +the maintainer states, that he/she is fine with the changes in principle, but > +didn't do a thorough review. The format is: > + > + Acked-by: name <email@domain> > + > +E.g.: > + > + Acked-by: Jane Doe <jane.doe@xxxxxxxxxxx> > + > +Including the `Acked-by:` tag in a patch is done under the same rules as for > +the `Reviewed-by:` tag, with the implied code area the maintainer who gave > the > +`Acked-by:` tag is responsible for. > + > +### Tested-by: > + > +The `Tested-by:` tag is another tag given by someone else. The one giving it > +confirms to have tested the patch without finding any functional issues. The > +format is: > + > + Tested-by: name <email@domain> Trailing white space. > + > +E.g.: > + > + Tested-by: Jane Doe <jane.doe@xxxxxxxxxxx> > + > +Including the `Tested-by:` tag in a patch is done under the same rules as for > +the `Reviewed-by:` tag, now limited to the patch not having been modified > +regarding code logic (having changed only coding style, comments, or message > +texts is fine). > + > +## Patch version history (change log), further comments > + > +When sending revised versions of a patch it is good practice to include a > +change log after a line containing only `---` (this line will result in the > +following text not being included in the commit message). This change log > +will help reviewers to spot which parts of the patch have changed. > Attributing > +changes due to reviewer comments will help the reviewer even more, e.g.: > + > + --- > + Changes in V2: I would use v2 (lowercase 'v'), because that's how git format-patch places the version in the subject line. > + - changed function foo() as requested by Jane Doe > + - code style fixed > + > +In some cases it might be desirable to add some more information for readers > +of the patch, like potential enhancements, other possible solutions, etc., > +which should not be part of the commit message. This information can be > +added after the `---` line, too. > + > +## Recipients of the patch > + > +A patch should always be sent **to** the xen-devel mailing list > <xen-devel@xxxxxxxxxxxxxxxxxxxx> and all maintainers of all touched code > areas should get a Missing newline. Thanks, Roger.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |