From owner-svn-doc-all@freebsd.org Tue Feb 12 22:48:40 2019 Return-Path: Delivered-To: svn-doc-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7305B14D5B75; Tue, 12 Feb 2019 22:48:40 +0000 (UTC) (envelope-from brooks@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 15C40874E2; Tue, 12 Feb 2019 22:48:40 +0000 (UTC) (envelope-from brooks@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 071051E7AE; Tue, 12 Feb 2019 22:48:40 +0000 (UTC) (envelope-from brooks@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x1CMmdpg035596; Tue, 12 Feb 2019 22:48:39 GMT (envelope-from brooks@FreeBSD.org) Received: (from brooks@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x1CMmdvP035595; Tue, 12 Feb 2019 22:48:39 GMT (envelope-from brooks@FreeBSD.org) Message-Id: <201902122248.x1CMmdvP035595@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: brooks set sender to brooks@FreeBSD.org using -f From: Brooks Davis Date: Tue, 12 Feb 2019 22:48:39 +0000 (UTC) To: doc-committers@freebsd.org, svn-doc-all@freebsd.org, svn-doc-head@freebsd.org Subject: svn commit: r52815 - head/en_US.ISO8859-1/articles/committers-guide X-SVN-Group: doc-head X-SVN-Commit-Author: brooks X-SVN-Commit-Paths: head/en_US.ISO8859-1/articles/committers-guide X-SVN-Commit-Revision: 52815 X-SVN-Commit-Repository: doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 15C40874E2 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.97 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.996,0]; NEURAL_HAM_SHORT(-0.98)[-0.976,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_HAM_LONG(-1.00)[-0.999,0] X-BeenThere: svn-doc-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire doc trees \(except for " user" , " projects" , and " translations" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Feb 2019 22:48:40 -0000 Author: brooks (src,ports committer) Date: Tue Feb 12 22:48:39 2019 New Revision: 52815 URL: https://svnweb.freebsd.org/changeset/doc/52815 Log: Committers Guide: Add a section encouraging pre-commit review. This is based on LLVM's Code Review policy. It differs it not requiring review for non-trivial changes. Reviewed by: bcr (verbal), allanjude, imp, jhb Approved by: core Differential Revision: https://reviews.freebsd.org/D16730 Modified: head/en_US.ISO8859-1/articles/committers-guide/article.xml Modified: head/en_US.ISO8859-1/articles/committers-guide/article.xml ============================================================================== --- head/en_US.ISO8859-1/articles/committers-guide/article.xml Mon Feb 11 18:34:32 2019 (r52814) +++ head/en_US.ISO8859-1/articles/committers-guide/article.xml Tue Feb 12 22:48:39 2019 (r52815) @@ -2380,6 +2380,101 @@ freebsd-mfc-after = 2 weeks + + Pre-Commit Review + + Code review is one way to increase the quality of software. + The following guidelines apply to commits to the + head (-CURRENT) branch of the + src repository. Other branches and the + ports and docs trees have + their own review policies, but these guidelines generally apply + to commits requiring review: + + + All non-trivial changes should be reviewed before they + are committed to the repository. + + + + Reviews may be conducted by email, in + Bugzilla, in + Phabricator, or by another + mechanism. Where possible, reviews should be public. + + + + The developer responsible for a code change is also + responsible for making all necessary review-related + changes. + + + + 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 looks good before it is committed. + So long as it is explicit, this can take whatever form makes + sense for the review method. + + + + Timeouts are not a substitute for review. + + + + 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: + + + + Review other people's patches. If you help out, + everybody will be more willing to do the same for you; + goodwill is our currency. + + + + 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. + + + + Ask for help on mailing lists, IRC, etc. Others + may be able to either help you directly, or suggest a + reviewer. + + + + 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. + + 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. + + + + 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. + + In some cases, no subject-matter expert may be available. + In those cases, a review by an experienced developer is + sufficient when coupled with appropriate testing. + + Commit Log Messages