| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 Oct 2023 09:30:07 -0700 From: Jakub Kicinski <kuba@...nel.org> To: davem@...emloft.net Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com, Jakub Kicinski <kuba@...nel.org>, andrew@...n.ch, jesse.brandeburg@...el.com, sd@...asysnail.net, horms@...ge.net.au Subject: [RFC] docs: netdev: encourage reviewers Add a section to our maintainer doc encouraging reviewers to chime in on the mailing list. The questions about "when is it okay to share feedback" keep coming up (most recently at netconf) and the answer is "pretty much always". The contents are partially based on a doc we wrote earlier and shared with the vendors (for the "driver review rotation"). Signed-off-by: Jakub Kicinski <kuba@...nel.org> -- CC: andrew@...n.ch CC: jesse.brandeburg@...el.com CC: sd@...asysnail.net CC: horms@...ge.net.au Sending as RFC for early round of reviews before I CC docs@ and expose this to potentially less constructive feedback :) --- Documentation/process/maintainer-netdev.rst | 43 +++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst index 09dcf6377c27..d5cbcfd44cf8 100644 --- a/Documentation/process/maintainer-netdev.rst +++ b/Documentation/process/maintainer-netdev.rst @@ -441,6 +441,49 @@ in a way which would break what would normally be considered uAPI. new ``netdevsim`` features must be accompanied by selftests under ``tools/testing/selftests/``. +Reviewer guidance +----------------- + +Reviewing other people's patches on the list is highly encouraged, +regardless of the level of expertise. Code reviews not only help +the maintainers but also help reviewers themselves to learn and +become part of the community. + +Reviewers can interact with the submissions by: asking questions +about the code, sharing relevant experience, suggesting changes, etc. +Asking questions is a particularly useful technique, for example rather +than stating: + +``I think there can be a deadlock between A and B here.`` + +it may be better to phrase the feedback as a question: + +``Could you explain what prevents deadlocks between A and B?`` + +After all, patch submissions should be clear both in terms of the code and +the commit message. + +Another technique useful in case of a disagreement is to ask for others +to chime in. I.e. if a discussion reaches a stalemate after a few exchanges, +calling for opinions of other reviewers or maintainers. Often those in +agreement with a reviewer remain silent unless called upon. +Opinion of multiple people carries exponentially more weight. + +There is no strict requirement to use specific tags like ``Reviewed-by``. +In fact reviews in plain English are more informative and encouraged +even when a tag is provided (e.g. "I looked at aspects A, B and C of this +submission and it looks good to me.") +Some form of a review message / reply is obviously necessary otherwise +maintainers will not know that the reviewer has looked at the patch at all! + +It's safe to assume that the maintainers know the community and the level +of expertise of the reviewers. The reviewers should not be concerned about +their comments impeding or derailing the patch flow. + +Last but not least patch review may become a negative process, focused +on pointing out problems. Please throw in a complement once in a while, +particularly for newbies! + Testimonials / feedback ----------------------- -- 2.41.0
Powered by blists - more mailing lists