diff options
Diffstat (limited to 'Documentation/process/7.AdvancedTopics.rst')
-rw-r--r-- | Documentation/process/7.AdvancedTopics.rst | 26 |
1 files changed, 22 insertions, 4 deletions
diff --git a/Documentation/process/7.AdvancedTopics.rst b/Documentation/process/7.AdvancedTopics.rst index 172733cff097..43291704338e 100644 --- a/Documentation/process/7.AdvancedTopics.rst +++ b/Documentation/process/7.AdvancedTopics.rst @@ -29,9 +29,9 @@ long document in its own right. Instead, the focus here will be on how git fits into the kernel development process in particular. Developers who wish to come up to speed with git will find more information at: - http://git-scm.com/ + https://git-scm.com/ - http://www.kernel.org/pub/software/scm/git/docs/user-manual.html + https://www.kernel.org/pub/software/scm/git/docs/user-manual.html and on various tutorials found on the web. @@ -55,7 +55,7 @@ server with git-daemon is relatively straightforward if you have a system which is accessible to the Internet. Otherwise, free, public hosting sites (Github, for example) are starting to appear on the net. Established developers can get an account on kernel.org, but those are not easy to come -by; see http://kernel.org/faq/ for more information. +by; see https://kernel.org/faq/ for more information. The normal git workflow involves the use of a lot of branches. Each line of development can be separated into a separate "topic branch" and @@ -125,7 +125,7 @@ can affect your ability to get trees pulled in the future. Quoting Linus: to trust things *without* then having to go and check every individual change by hand. -(http://lwn.net/Articles/224135/). +(https://lwn.net/Articles/224135/). To avoid this kind of situation, ensure that all patches within a given branch stick closely to the associated topic; a "driver fixes" branch @@ -146,6 +146,7 @@ pull. The git request-pull command can be helpful in this regard; it will format the request as other developers expect, and will also check to be sure that you have remembered to push those changes to the public server. +.. _development_advancedtopics_reviews: Reviewing patches ----------------- @@ -167,6 +168,12 @@ comments as questions rather than criticisms. Asking "how does the lock get released in this path?" will always work better than stating "the locking here is wrong." +Another technique that is useful in case of a disagreement is to ask for others +to chime in. If a discussion reaches a stalemate after a few exchanges, +then call for opinions of other reviewers or maintainers. Often those in +agreement with a reviewer remain silent unless called upon. +The opinion of multiple people carries exponentially more weight. + Different developers will review code from different points of view. Some are mostly concerned with coding style and whether code lines have trailing white space. Others will focus primarily on whether the change implemented @@ -176,3 +183,14 @@ security issues, duplication of code found elsewhere, adequate documentation, adverse effects on performance, user-space ABI changes, etc. All types of review, if they lead to better code going into the kernel, are welcome and worthwhile. + +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 or reply is obviously necessary otherwise +maintainers will not know that the reviewer has looked at the patch at all! + +Last but not least patch review may become a negative process, focused +on pointing out problems. Please throw in a compliment once in a while, +particularly for newbies! |