Page MenuHomeFreeBSD
Files F137316383

D16730.id47185.diff
No OneTemporary
Actions

D16730.id47185.diff
View Options

Index: en_US.ISO8859-1/articles/committers-guide/article.xml
===================================================================
--- en_US.ISO8859-1/articles/committers-guide/article.xml
+++ en_US.ISO8859-1/articles/committers-guide/article.xml
@@ -2302,6 +2302,96 @@
</sect2>
</sect1>
+ <sect1 xml:id="pre-commit-review">
+ <title>Pre-Commit Review</title>
+
+ <para>Code review is one way to increase the quality of software.
+ The following guidelines apply to commits to the
+ <literal>src</literal> repository. The <literal>ports</literal>
+ and <literal>docs</literal> trees have their own review
+ policies, but these guidelines generally apply to commits
+ requiring review:</para>
+ <itemizedlist>
+ <listitem>
+ <para>All non-trivial changes should be reviewed before they
+ are committed to the repository.</para>
+ </listitem>
+
+ <listitem>
+ <para>Reviews may be conducted by email, in
+ <application>Bugzilla</application>, in
+ <application>Phabrictor</application>, or by other
+ mechanism. Where possible, reviews should be public.</para>
+ </listitem>
+
+ <listitem>
+ <para>The developer responsible for a code change is also
+ responsible for making all necessary review-related
+ changes.</para>
+ </listitem>
+
+ <listitem>
+ <para>Code review can be an iterative process, which continues
+ until the patch is ready to be committed. Specifically,
+ once a patch is sent out for review, it should receive an
+ explicit <quote>looks good</quote> before it is committed.
+ So long as it is explicit, this can take whatever form makes
+ sense for the review method.</para>
+ </listitem>
+ </itemizedlist>
+
+ <para>Sometimes code reviews will take longer than you would hope
+ for, especially for larger features. Accepted ways to speed up
+ review times for your patches are:</para>
+
+ <itemizedlist>
+ <listitem>
+ <para>Review other people's patches. If you help out,
+ everybody will be more willing to do the same for you;
+ goodwill is our currency.</para>
+ </listitem>
+
+ <listitem>
+ <para>Ping the patch. If it is urgent, provide reasons why
+ it is important to you to get this patch landed and ping
+ it every couple of days. If it is not urgent, the common
+ courtesy ping rate is one week. Remember that you are
+ asking for valuable time from other professional
+ developers.</para>
+ </listitem>
+
+ <listitem>
+ <para>Ask for help on mailing lists, IRC, etc. Others
+ may be able to either help you directly, or suggest a
+ reviewer.</para>
+ </listitem>
+
+ <listitem>
+ <para>Split your patch into multiple smaller patches that
+ build on each other. The smaller your patch, the higher
+ the probability that somebody will take a quick look at
+ it.</para>
+
+ <para>When making large changes, it is helpful to keep this
+ in mind from the beginning of the effort as breaking large
+ changes into smaller ones is often difficult after the
+ fact.</para>
+ </listitem>
+ </itemizedlist>
+
+ <para>Developers should participate in code reviews as both
+ reviewers and reviewees. If someone is kind enough to review
+ your code, you should return the favor for someone else.
+ Note that while anyone is welcome to review and give feedback
+ on a patch, only an appropriate subject-matter expert can
+ approve a change. This will usually be a committer who works
+ with the code in question on a regular basis.</para>
+
+ <para>In some cases, no subject-matter expert may be available.
+ In those cases, a review by an experienced developer is
+ when coupled with sufficient testing.</para>
+ </sect1>
+
<sect1 xml:id="commit-log-message">
<title>Commit Log Messages</title>

File Metadata

Mime Type
text/plain
Expires
Sun, Nov 23, 9:38 AM (7 h, 40 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
26010504
Default Alt Text
D16730.id47185.diff (3 KB)

Event Timeline