Page MenuHomeFreeBSD
Files F142284016

D16730.id46741.diff
No OneTemporary
Actions

D16730.id46741.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,89 @@
</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 FreeBSD Project encourages all committers to follow these
+ guidelines:</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 anyone is welcome to review and give feedback on a patch,
+ but ultimately 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>
+ </sect1>
+
<sect1 xml:id="commit-log-message">
<title>Commit Log Messages</title>

File Metadata

Mime Type
text/plain
Expires
Mon, Jan 19, 4:12 AM (20 h, 26 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
27730424
Default Alt Text
D16730.id46741.diff (3 KB)

Event Timeline