From: Bjorn Helgaas <helgaas@kernel.org>
To: linux-pci@vger.kernel.org
Subject: Make Bjorn's life easier (and grease the path of your patch)
Date: Thu, 26 Oct 2017 17:37:01 -0500 [thread overview]
Message-ID: <20171026223701.GA25649@bhelgaas-glaptop.roam.corp.google.com> (raw)
This is a list of things I look at when reviewing patches. Most of
the technical things are obvious or covered elsewhere, so this list is
mostly cosmetic things. I hesitate to post it because (a) it feels so
trivial, (b) most of these things seem obvious as well, and (c) I
normally clean them up silently in the process of applying patches.
But things that seem obvious to the receiver are not always obvious to
the sender, and I do spend a lot of time on this cleanup. When I get
patches that need a lot of it, I put off looking at them because I
figure the author didn't really care that much. So if you *do* care,
and you pay attention to these details, I'm more likely to handle your
patch quickly.
Write the patch:
- Make each patch small but complete by itself. It's trivial for me
to squash patches together if they get *too* small.
- Make sure the kernel builds after every patch. You can't
introduce a problem in one patch and fix it in a subsequent patch.
If one of the autobuilders finds a build issue, I'll revert the
patch unless you send a fix.
- Please send whitespace and coding style fixes, but don't mix them
with other substantive changes. It's easier for people to
backport fixes if whitespace changes are at the end of a series.
- Wrap code and comments to fit in 80 columns. Exception: I prefer
printk strings to be in one piece for searchability, so don't
split them to make them fit in 80 columns.
- Follow the existing style for code, names, and indentation. If I
can tell that more than one person contributed to the file, you're
doing something wrong.
Write the changelog title (first line of the changelog):
- Follow the existing convention, i.e., run "git log --oneline
<file>" and make yours match in format, capitalization, and
sentence structure. For example, native host bridge driver patch
titles look like this:
PCI: altera: Fix platform_get_irq() error handling
PCI: vmd: Remove IRQ affinity so we can allocate more IRQs
PCI: mediatek: Add MSI support for MT2712 and MT7622
PCI: rockchip: Remove IRQ domain if probe fails
- Write a complete sentence, starting with a capitalized verb.
- Include specific details, e.g., write "Add XYZ controller support"
instead of "add support for new generation controller".
- Do not include a trailing period.
Write the changelog:
- Make the changelog readable without the title. The changelog is
not a continuation of the title, so it should make sense by
itself.
- Explain the change (not just "Fix this problem"), but do it as
concisely as possible. I don't want a book, but I do appreciate
the function names, spec references, etc. that help a reviewer
verify the change.
- Capitalize initialisms ("PCI", "IRQ", "ID", "MSI", etc) in all
English text, including title, changelog, and comments.
- Include "()" after function names and "[]" after table array
names.
- Include dmesg output and stack trace when relevant. Prune details
that aren't relevant, e.g., you can usually remove timestamps and
function addresses. The objective is to concisely illustrate the
issue and make it discoverable by search engines.
- Remove tabs from the changelog because "git log" indents the
changelog and things aligned with tabs won't stay aligned.
- Wrap changelogs to fit in 80 columns when shown by "git show",
which adds 4 spaces. I use "textwidth=75" in vim.
- Order tags as suggested by Ingo [1] (extended):
Fixes:
Link:
Reported-by:
Tested-by:
Signed-off-by: (author)
Signed-off-by: (chain)
Reviewed-by:
Acked-by:
Cc: stable@vger.kernel.org # 3.4+
Cc: (other)
- Include a "Fixes:" reference when you're fixing a previous commit
and copy the author of the previous commit. This helps people
figure out where a change needs to be backported.
- Include specific commit references when possible, e.g.,
'e77f847df54c ("PCI: rockchip: Add Rockchip PCIe controller
support")'. I use this alias to generate them:
alias gsr='git --no-pager show -s --abbrev-commit --abbrev=12 --pretty=format:"%h (\"%s\")%n"'
- Include bugzilla URLs if available (kernel.org bugzilla
preferred), e.g.,
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1409201
- Include problem report URLs (http://lkml.kernel.org/r/ preferred),
e.g.,
Link: http://lkml.kernel.org/r/4bcbcbc1-7c79-09f0-5071-bc2f53bf6574@kernel.dk
- Include specific spec references when possible, e.g., "PCIe r3.1,
sec 7.8.2". If you're talking about something mentioned in the
spec, use the same name and capitalization as the spec.
- Include a "CC: stable@vger.kernel.org" tag if you want one, and
indicate which kernels need it.
Post the patch:
- Copy linux-pci@vger.kernel.org. I don't apply patches that
haven't appeared on the mailing list, even if you send them to me
directly. This is partly to make sure everyone has a chance to
review it and partly because I use the Patchwork tracker [2],
which only sees things on the mailing list.
- If you send more than one patch and they're related, always
include a "[0/n]" cover letter. This makes it easy for me to
reply saying "I applied this series." I use "stg -e -v v1
--to=... patch1..patchN".
- If you post a new version, please make sure it includes "[v2]" or
whatever in the subject line. If it's a series, I want a new
version of the entire series. I don't want updates of individual
patches within the series -- that's too hard to manage. It's
perfectly fine if some patches in a v2 series are the same as in
the initial posting.
- If you're really gung-ho, you can go to Patchwork [2] and mark
your superseded patches as "Superseded" so I don't have to do that
myself.
- If you want the patch in the current release, include a cover
letter and tell me that. Otherwise, I assume all patches are
intended for the next merge window.
[1] http://lkml.kernel.org/r/20120711080446.GA17713@gmail.com
[2] https://patchwork.ozlabs.org/project/linux-pci/list/
next reply other threads:[~2017-10-26 22:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-26 22:37 Bjorn Helgaas [this message]
2023-09-21 15:14 ` Make Bjorn's life easier (and grease the path of your patch) Lukas Wunner
2023-09-21 16:27 ` Bjorn Helgaas
2023-09-21 18:44 ` Krzysztof Wilczyński
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171026223701.GA25649@bhelgaas-glaptop.roam.corp.google.com \
--to=helgaas@kernel.org \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.