You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2015/04/24 01:14:46 UTC
svn commit: r1675732 - in /mesos/site: publish/ publish/documentation/
publish/documentation/committers-guide/ publish/documentation/committing/
publish/documentation/effective-code-reviewing/
publish/documentation/latest/ publish/documentation/latest/...
Author: bmahler
Date: Thu Apr 23 23:14:45 2015
New Revision: 1675732
URL: http://svn.apache.org/r1675732
Log:
Documentation updates for reviews and commits.
Added:
mesos/site/publish/documentation/committing/
mesos/site/publish/documentation/committing/index.html
mesos/site/publish/documentation/effective-code-reviewing/
mesos/site/publish/documentation/effective-code-reviewing/index.html
mesos/site/publish/documentation/latest/committing/
mesos/site/publish/documentation/latest/committing/index.html
mesos/site/publish/documentation/latest/effective-code-reviewing/
mesos/site/publish/documentation/latest/effective-code-reviewing/index.html
mesos/site/source/documentation/latest/committing.md
mesos/site/source/documentation/latest/effective-code-reviewing.md
Removed:
mesos/site/publish/documentation/committers-guide/
mesos/site/publish/documentation/latest/committers-guide/
mesos/site/publish/documentation/latest/mesos-code-internals/
mesos/site/publish/documentation/latest/mesos-developers-guide/
mesos/site/publish/documentation/mesos-code-internals/
mesos/site/publish/documentation/mesos-developers-guide/
mesos/site/source/documentation/latest/committers-guide.md
mesos/site/source/documentation/latest/mesos-code-internals.md
mesos/site/source/documentation/latest/mesos-developers-guide.md
Modified:
mesos/site/publish/documentation/index.html
mesos/site/publish/documentation/latest/index.html
mesos/site/publish/sitemap.xml
mesos/site/source/documentation/latest.html.md
Added: mesos/site/publish/documentation/committing/index.html
URL: http://svn.apache.org/viewvc/mesos/site/publish/documentation/committing/index.html?rev=1675732&view=auto
==============================================================================
--- mesos/site/publish/documentation/committing/index.html (added)
+++ mesos/site/publish/documentation/committing/index.html Thu Apr 23 23:14:45 2015
@@ -0,0 +1,139 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta charset="utf-8">
+ <title></title>
+ <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+ <link href="//netdna.bootstrapcdn.com/bootstrap/3.1.1/css/bootstrap.min.css" rel="stylesheet">
+ <link rel="alternate" type="application/atom+xml" title="Apache Mesos Blog" href="/blog/feed.xml">
+
+ <link href="../../assets/css/main.css" media="screen" rel="stylesheet" type="text/css" />
+
+
+
+ <!-- Google Analytics Magic -->
+ <script type="text/javascript">
+ var _gaq = _gaq || [];
+ _gaq.push(['_setAccount', 'UA-20226872-1']);
+ _gaq.push(['_setDomainName', 'apache.org']);
+ _gaq.push(['_trackPageview']);
+
+ (function() {
+ var ga = document.createElement('script'); ga.type = 'text/javascript'; ga.async = true;
+ ga.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.google-analytics.com/ga.js';
+ var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(ga, s);
+ })();
+ </script>
+ </head>
+ <body>
+ <!-- magical breadcrumbs -->
+ <div class="topnav">
+ <ul class="breadcrumb">
+ <li>
+ <div class="dropdown">
+ <a data-toggle="dropdown" href="#">Apache Software Foundation <span class="caret"></span></a>
+ <ul class="dropdown-menu" role="menu" aria-labelledby="dLabel">
+ <li><a href="http://www.apache.org">Apache Homepage</a></li>
+ <li><a href="http://www.apache.org/licenses/">License</a></li>
+ <li><a href="http://www.apache.org/foundation/sponsorship.html">Sponsorship</a></li>
+ <li><a href="http://www.apache.org/foundation/thanks.html">Thanks</a></li>
+ <li><a href="http://www.apache.org/security/">Security</a></li>
+ </ul>
+ </div>
+ </li>
+ <li><a href="http://mesos.apache.org">Apache Mesos</a></li>
+
+
+ <li><a href="/documentation
+/">Documentation
+</a></li>
+
+
+ </ul><!-- /breadcrumb -->
+ </div>
+
+ <!-- navbar excitement -->
+ <div class="navbar navbar-static-top" role="navigation">
+ <div class="navbar-inner">
+ <div class="container">
+ <a href="/" class="logo"><img src="/assets/img/mesos_logo.png" alt="Apache Mesos logo" /></a>
+ <div class="nav-collapse">
+ <ul class="nav nav-pills navbar-right">
+ <li><a href="/gettingstarted/">Getting Started</a></li>
+ <li><a href="/documentation/latest/">Documentation</a></li>
+ <li><a href="/downloads/">Downloads</a></li>
+ <li><a href="/community/">Community</a></li>
+ </ul>
+ </div>
+ </div>
+ </div>
+ </div><!-- /.navbar -->
+
+ <div class="container">
+
+ <div class="row-fluid">
+ <div class="col-md-4">
+ <h4>If you're new to Mesos</h4>
+ <p>See the <a href="/gettingstarted/">getting started</a> page for more information about downloading, building, and deploying Mesos.</p>
+
+ <h4>If you'd like to get involved or you're looking for support</h4>
+ <p>See our <a href="/community/">community</a> page for more details.</p>
+ </div>
+ <div class="col-md-8">
+ <h1>Committing</h1>
+
+<p>Only committers have the ability to commit your change, so make sure you
+work with one during code review to get your changes committed.</p>
+
+<p>If you are a committer, here are some guidelines for committing changes:</p>
+
+<ol>
+<li><strong>Follow the format of commit messages</strong>: we currently do not leverage
+any tooling to enforce the format of messages, so:
+
+<ol type="a">
+<li>Be clear and explicit in the commit message.</li>
+<li>Include the link to the review (this will be done automatically if
+using <code>support/post-reviews.py</code> for your own changes and
+<code>support/apply-review.sh</code> for committing changes from others).
+<strong>Be sure to clean up the commit message when pulling in changes
+from others.</strong></li>
+<li>Use 72 character columns. Note that we don’t always have a 50
+character or less summary because that restriction tends to cause
+people to write poorly.</li>
+</ol>
+</li>
+<li><strong>Keep the ‘Author’ intact</strong>: <code>support/apply-review.sh</code> will handle
+this for you, but be careful when rebasing or ammending.</li>
+<li><strong>Never ever commit a merge</strong>: always rebase instead, as appropriate.
+Likewise, never ‘force push’.</li>
+<li><strong>Don’t break the build</strong>: we support Linux and Mac OS X, however not
+all configurations are being tested in Jenkins, so be aware of that.
+Also, pay attention to the Jenkins review bot if it flags a review as
+breaking the build. Note that if you do break the build, the fixes are
+often small and inconsequential so don’t worry about going through a
+review cycle for that, just fix things (but don’t take that as a license
+to “wait until the build fails to fix things”).</li>
+</ol>
+
+
+ </div>
+</div>
+
+
+ <hr>
+
+ <!-- footer -->
+ <div class="footer">
+ <p>© 2012-2014 <a href="http://apache.org">The Apache Software Foundation</a>.
+ Apache Mesos, the Apache feather logo, and the Apache Mesos project logo are trademarks of The Apache Software Foundation.<p>
+ </div><!-- /footer -->
+
+ </div> <!-- /container -->
+
+ <!-- JS -->
+ <script src="//code.jquery.com/jquery-1.11.0.min.js" type="text/javascript"></script>
+ <script src="//netdna.bootstrapcdn.com/bootstrap/3.1.1/js/bootstrap.min.js" type="text/javascript"></script>
+ </body>
+</html>
\ No newline at end of file
Added: mesos/site/publish/documentation/effective-code-reviewing/index.html
URL: http://svn.apache.org/viewvc/mesos/site/publish/documentation/effective-code-reviewing/index.html?rev=1675732&view=auto
==============================================================================
--- mesos/site/publish/documentation/effective-code-reviewing/index.html (added)
+++ mesos/site/publish/documentation/effective-code-reviewing/index.html Thu Apr 23 23:14:45 2015
@@ -0,0 +1,204 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta charset="utf-8">
+ <title></title>
+ <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+ <link href="//netdna.bootstrapcdn.com/bootstrap/3.1.1/css/bootstrap.min.css" rel="stylesheet">
+ <link rel="alternate" type="application/atom+xml" title="Apache Mesos Blog" href="/blog/feed.xml">
+
+ <link href="../../assets/css/main.css" media="screen" rel="stylesheet" type="text/css" />
+
+
+
+ <!-- Google Analytics Magic -->
+ <script type="text/javascript">
+ var _gaq = _gaq || [];
+ _gaq.push(['_setAccount', 'UA-20226872-1']);
+ _gaq.push(['_setDomainName', 'apache.org']);
+ _gaq.push(['_trackPageview']);
+
+ (function() {
+ var ga = document.createElement('script'); ga.type = 'text/javascript'; ga.async = true;
+ ga.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.google-analytics.com/ga.js';
+ var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(ga, s);
+ })();
+ </script>
+ </head>
+ <body>
+ <!-- magical breadcrumbs -->
+ <div class="topnav">
+ <ul class="breadcrumb">
+ <li>
+ <div class="dropdown">
+ <a data-toggle="dropdown" href="#">Apache Software Foundation <span class="caret"></span></a>
+ <ul class="dropdown-menu" role="menu" aria-labelledby="dLabel">
+ <li><a href="http://www.apache.org">Apache Homepage</a></li>
+ <li><a href="http://www.apache.org/licenses/">License</a></li>
+ <li><a href="http://www.apache.org/foundation/sponsorship.html">Sponsorship</a></li>
+ <li><a href="http://www.apache.org/foundation/thanks.html">Thanks</a></li>
+ <li><a href="http://www.apache.org/security/">Security</a></li>
+ </ul>
+ </div>
+ </li>
+ <li><a href="http://mesos.apache.org">Apache Mesos</a></li>
+
+
+ <li><a href="/documentation
+/">Documentation
+</a></li>
+
+
+ </ul><!-- /breadcrumb -->
+ </div>
+
+ <!-- navbar excitement -->
+ <div class="navbar navbar-static-top" role="navigation">
+ <div class="navbar-inner">
+ <div class="container">
+ <a href="/" class="logo"><img src="/assets/img/mesos_logo.png" alt="Apache Mesos logo" /></a>
+ <div class="nav-collapse">
+ <ul class="nav nav-pills navbar-right">
+ <li><a href="/gettingstarted/">Getting Started</a></li>
+ <li><a href="/documentation/latest/">Documentation</a></li>
+ <li><a href="/downloads/">Downloads</a></li>
+ <li><a href="/community/">Community</a></li>
+ </ul>
+ </div>
+ </div>
+ </div>
+ </div><!-- /.navbar -->
+
+ <div class="container">
+
+ <div class="row-fluid">
+ <div class="col-md-4">
+ <h4>If you're new to Mesos</h4>
+ <p>See the <a href="/gettingstarted/">getting started</a> page for more information about downloading, building, and deploying Mesos.</p>
+
+ <h4>If you'd like to get involved or you're looking for support</h4>
+ <p>See our <a href="/community/">community</a> page for more details.</p>
+ </div>
+ <div class="col-md-8">
+ <h1>Effective Code Reviewing</h1>
+
+<h3>Preparing for Review</h3>
+
+<p>We’ve found that clear, clean, small, independent, incremental changes are much
+easier to review thoroughly, and much easier to get committed. Here are some tips
+to consider before sending review requests:</p>
+
+<ol>
+<li><strong>Use up front discussion</strong>: Surprising a reviewer is likely to lead to
+wasted effort if the overall design needs changing. Try to get a reviewer
+on the same page before sending them a review request.</li>
+<li><strong>Think about how to break up your patch</strong>: Did you make any changes
+that could could be reviewed in independent patches? We use
+<code>support/post-reviews.py</code>, which makes it easy to create “chains” of
+reviews based on commits. Become familiar with interactive rebasing
+(<code>git rebase -i</code>) to split up commits.</li>
+<li><strong>Provide context for the change</strong>: Make the motivations for the
+change clear in the review request, so the reviewer is not left
+guessing. It is highly recommended to attach a JIRA issue with your
+review for additional context.</li>
+<li><strong>Follow the <a href="http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/">style guide</a>
+and the style of code around you</strong>.</li>
+<li><strong>Do a self-review of your changes before publishing</strong>: Approach it
+from the perspective of a reviewer with no context. Is it easy to figure
+out the motivations for the change? Does it fit in with the rest of the
+code style? Are there unnecessary changes that crept in? Did you do any
+testing of the change?</li>
+</ol>
+
+
+<h3>Reviewing</h3>
+
+<ol>
+<li><strong>Do high level reviewing before low-level reviewing</strong>: Consider the
+big picture before you get into any low-level details in the code.
+For example, do you understand what motivated the change? Why the
+author chose the particular solution? Does it “feel” more complicated
+than necessary? If so, invest the time to consider if there are
+simpler approaches to the problem, or whether the scope of the work
+can be reduced. These should all be addressed before doing a
+line-by-line thorough code review, as the latter is subject to a
+lot of review back-and-forth in the face of changes to the overall
+approach.</li>
+<li><strong>Be patient, thoughtful, and respectful</strong>: when providing (a) feedback
+on reviews and (b) commenting on feedback. Practice
+<a href="http://blog.codinghorror.com/the-ten-commandments-of-egoless-programming/">ego-less programming</a>,
+this is not meant to be a sparring match! A prerequisite to being a good
+reviewee is acknowledging that ‘writing is hard’! The reviewee should give
+the reviewer the benefit of the doubt that the reviewer has a real concern
+even if they can’t articulate it well. And the reviewee should be prepared
+to anticipate all of the reviewers concerns and be thoughtful about why
+they decided to do something a particular way (especially if it deviates
+from a standard in the codebase). On the flip side, the reviewer should
+not use the fact that ‘writing is hard’ as a crutch for giving hasty
+feedback. The reviewer should invest time and energy trying to explain
+their concerns clearly and carefully. It’s a two-way street!</li>
+<li><strong>Resolve issues before committing</strong>: we tend to give a ‘ship it’ even when
+we think there are some issues that need correcting. Sometimes a particular
+issue will require more discussion and sometimes we take that discussion to
+IM or IRC or emails to expedite the process. It’s important, however, that
+we publicly capture any collaborations/discussions that happened in those
+means. Making the discussion public also helps involve others that would
+have liked to get involved but weren’t invited.
+
+<ol type="a">
+<li>If the reviewer and reviewee are having problems resolving a particular
+“confrontational” issue then both parties should consider communicating
+directly or asking another reviewer to participate. <strong>We’re all here to
+build the highest quality code possible, and we should leverage one
+another to do so.</strong></li>
+<li>When an issue is “Dropped” by the reviewee, the expectation is that there
+must be a reply to the issue indicating why it was dropped. A silent “Drop”
+is discouraged.</li>
+<li>If an issue is marked as “Resolved”, the expectation is that the diff has
+been updated in accordance (more or less) with the reviewer’s comment. If
+there are significant changes, a reply to the issue with a comment is
+greatly appreciated.</li>
+</ol>
+</li>
+<li><strong>Be explicit about asking for more feedback</strong>: feel free to update reviews
+as often as you like but recognize that in many cases it’s ambiguous for
+reviewers when a review is back into a “ready” state so the reviewee should
+feel free to ping the reviewers via email when they’re ready.</li>
+<li><strong>For reviewees, wait for a ‘ship it’</strong>: We often use the original developer
+or current “custodian” of some particular code to be the reviewer and add others
+to get more feedback or as an FYI. Feel free to explicitly call out that you’re
+adding some people just as an FYI in the ‘Description’. It’s worth mentioning
+that we often reach out directly to one another about a particular change/feature
+before we even start coding. A little context goes a long way to streamlining the
+review process.</li>
+<li><strong>For reviewers, be thorough when giving a ‘ship it’</strong>: understand that
+reviewing requires a substantial investment of time and focus:
+
+<ol type="a">
+<li>You are expected to understand and support code that you’re giving a ‘ship it’ to.</li>
+<li>You are expected to be accountable for the quality of the code you’ve given a ‘ship it’ to.</li>
+</ol>
+</li>
+</ol>
+
+
+ </div>
+</div>
+
+
+ <hr>
+
+ <!-- footer -->
+ <div class="footer">
+ <p>© 2012-2014 <a href="http://apache.org">The Apache Software Foundation</a>.
+ Apache Mesos, the Apache feather logo, and the Apache Mesos project logo are trademarks of The Apache Software Foundation.<p>
+ </div><!-- /footer -->
+
+ </div> <!-- /container -->
+
+ <!-- JS -->
+ <script src="//code.jquery.com/jquery-1.11.0.min.js" type="text/javascript"></script>
+ <script src="//netdna.bootstrapcdn.com/bootstrap/3.1.1/js/bootstrap.min.js" type="text/javascript"></script>
+ </body>
+</html>
\ No newline at end of file
Modified: mesos/site/publish/documentation/index.html
URL: http://svn.apache.org/viewvc/mesos/site/publish/documentation/index.html?rev=1675732&r1=1675731&r2=1675732&view=diff
==============================================================================
--- mesos/site/publish/documentation/index.html (original)
+++ mesos/site/publish/documentation/index.html Thu Apr 23 23:14:45 2015
@@ -138,7 +138,8 @@
<ul>
<li><a href="/documentation/latest/reporting-a-bug/">Reporting an Issue, Improvement, or Feature</a> for getting started with JIRA.</li>
<li><a href="/documentation/latest/submitting-a-patch/">Submitting a Patch</a> for getting started with ReviewBoard, and our tooling around it.</li>
-<li><a href="/documentation/latest/committers-guide/">Committer’s Guide</a> a guiding document for etiquette in reviews and commits.</li>
+<li><a href="/documentation/latest/committers-guide/">Effective Code Reviewing</a> guidelines, tips, and learnings for how to do effective code reviews.</li>
+<li><a href="/documentation/latest/committing/">Committing</a> guidelines for committing changes.</li>
<li><a href="/documentation/latest/mesos-c++-style-guide/">C++ Style Guide</a></li>
<li><a href="/documentation/latest/mesos-roadmap/">Development Roadmap</a></li>
<li><a href="/documentation/latest/release-guide/">Release Guide</a></li>
Added: mesos/site/publish/documentation/latest/committing/index.html
URL: http://svn.apache.org/viewvc/mesos/site/publish/documentation/latest/committing/index.html?rev=1675732&view=auto
==============================================================================
--- mesos/site/publish/documentation/latest/committing/index.html (added)
+++ mesos/site/publish/documentation/latest/committing/index.html Thu Apr 23 23:14:45 2015
@@ -0,0 +1,139 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta charset="utf-8">
+ <title></title>
+ <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+ <link href="//netdna.bootstrapcdn.com/bootstrap/3.1.1/css/bootstrap.min.css" rel="stylesheet">
+ <link rel="alternate" type="application/atom+xml" title="Apache Mesos Blog" href="/blog/feed.xml">
+
+ <link href="../../../assets/css/main.css" media="screen" rel="stylesheet" type="text/css" />
+
+
+
+ <!-- Google Analytics Magic -->
+ <script type="text/javascript">
+ var _gaq = _gaq || [];
+ _gaq.push(['_setAccount', 'UA-20226872-1']);
+ _gaq.push(['_setDomainName', 'apache.org']);
+ _gaq.push(['_trackPageview']);
+
+ (function() {
+ var ga = document.createElement('script'); ga.type = 'text/javascript'; ga.async = true;
+ ga.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.google-analytics.com/ga.js';
+ var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(ga, s);
+ })();
+ </script>
+ </head>
+ <body>
+ <!-- magical breadcrumbs -->
+ <div class="topnav">
+ <ul class="breadcrumb">
+ <li>
+ <div class="dropdown">
+ <a data-toggle="dropdown" href="#">Apache Software Foundation <span class="caret"></span></a>
+ <ul class="dropdown-menu" role="menu" aria-labelledby="dLabel">
+ <li><a href="http://www.apache.org">Apache Homepage</a></li>
+ <li><a href="http://www.apache.org/licenses/">License</a></li>
+ <li><a href="http://www.apache.org/foundation/sponsorship.html">Sponsorship</a></li>
+ <li><a href="http://www.apache.org/foundation/thanks.html">Thanks</a></li>
+ <li><a href="http://www.apache.org/security/">Security</a></li>
+ </ul>
+ </div>
+ </li>
+ <li><a href="http://mesos.apache.org">Apache Mesos</a></li>
+
+
+ <li><a href="/documentation
+/">Documentation
+</a></li>
+
+
+ </ul><!-- /breadcrumb -->
+ </div>
+
+ <!-- navbar excitement -->
+ <div class="navbar navbar-static-top" role="navigation">
+ <div class="navbar-inner">
+ <div class="container">
+ <a href="/" class="logo"><img src="/assets/img/mesos_logo.png" alt="Apache Mesos logo" /></a>
+ <div class="nav-collapse">
+ <ul class="nav nav-pills navbar-right">
+ <li><a href="/gettingstarted/">Getting Started</a></li>
+ <li><a href="/documentation/latest/">Documentation</a></li>
+ <li><a href="/downloads/">Downloads</a></li>
+ <li><a href="/community/">Community</a></li>
+ </ul>
+ </div>
+ </div>
+ </div>
+ </div><!-- /.navbar -->
+
+ <div class="container">
+
+ <div class="row-fluid">
+ <div class="col-md-4">
+ <h4>If you're new to Mesos</h4>
+ <p>See the <a href="/gettingstarted/">getting started</a> page for more information about downloading, building, and deploying Mesos.</p>
+
+ <h4>If you'd like to get involved or you're looking for support</h4>
+ <p>See our <a href="/community/">community</a> page for more details.</p>
+ </div>
+ <div class="col-md-8">
+ <h1>Committing</h1>
+
+<p>Only committers have the ability to commit your change, so make sure you
+work with one during code review to get your changes committed.</p>
+
+<p>If you are a committer, here are some guidelines for committing changes:</p>
+
+<ol>
+<li><strong>Follow the format of commit messages</strong>: we currently do not leverage
+any tooling to enforce the format of messages, so:
+
+<ol type="a">
+<li>Be clear and explicit in the commit message.</li>
+<li>Include the link to the review (this will be done automatically if
+using <code>support/post-reviews.py</code> for your own changes and
+<code>support/apply-review.sh</code> for committing changes from others).
+<strong>Be sure to clean up the commit message when pulling in changes
+from others.</strong></li>
+<li>Use 72 character columns. Note that we don’t always have a 50
+character or less summary because that restriction tends to cause
+people to write poorly.</li>
+</ol>
+</li>
+<li><strong>Keep the ‘Author’ intact</strong>: <code>support/apply-review.sh</code> will handle
+this for you, but be careful when rebasing or ammending.</li>
+<li><strong>Never ever commit a merge</strong>: always rebase instead, as appropriate.
+Likewise, never ‘force push’.</li>
+<li><strong>Don’t break the build</strong>: we support Linux and Mac OS X, however not
+all configurations are being tested in Jenkins, so be aware of that.
+Also, pay attention to the Jenkins review bot if it flags a review as
+breaking the build. Note that if you do break the build, the fixes are
+often small and inconsequential so don’t worry about going through a
+review cycle for that, just fix things (but don’t take that as a license
+to “wait until the build fails to fix things”).</li>
+</ol>
+
+
+ </div>
+</div>
+
+
+ <hr>
+
+ <!-- footer -->
+ <div class="footer">
+ <p>© 2012-2014 <a href="http://apache.org">The Apache Software Foundation</a>.
+ Apache Mesos, the Apache feather logo, and the Apache Mesos project logo are trademarks of The Apache Software Foundation.<p>
+ </div><!-- /footer -->
+
+ </div> <!-- /container -->
+
+ <!-- JS -->
+ <script src="//code.jquery.com/jquery-1.11.0.min.js" type="text/javascript"></script>
+ <script src="//netdna.bootstrapcdn.com/bootstrap/3.1.1/js/bootstrap.min.js" type="text/javascript"></script>
+ </body>
+</html>
\ No newline at end of file
Added: mesos/site/publish/documentation/latest/effective-code-reviewing/index.html
URL: http://svn.apache.org/viewvc/mesos/site/publish/documentation/latest/effective-code-reviewing/index.html?rev=1675732&view=auto
==============================================================================
--- mesos/site/publish/documentation/latest/effective-code-reviewing/index.html (added)
+++ mesos/site/publish/documentation/latest/effective-code-reviewing/index.html Thu Apr 23 23:14:45 2015
@@ -0,0 +1,204 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta charset="utf-8">
+ <title></title>
+ <meta name="viewport" content="width=device-width, initial-scale=1.0">
+
+ <link href="//netdna.bootstrapcdn.com/bootstrap/3.1.1/css/bootstrap.min.css" rel="stylesheet">
+ <link rel="alternate" type="application/atom+xml" title="Apache Mesos Blog" href="/blog/feed.xml">
+
+ <link href="../../../assets/css/main.css" media="screen" rel="stylesheet" type="text/css" />
+
+
+
+ <!-- Google Analytics Magic -->
+ <script type="text/javascript">
+ var _gaq = _gaq || [];
+ _gaq.push(['_setAccount', 'UA-20226872-1']);
+ _gaq.push(['_setDomainName', 'apache.org']);
+ _gaq.push(['_trackPageview']);
+
+ (function() {
+ var ga = document.createElement('script'); ga.type = 'text/javascript'; ga.async = true;
+ ga.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.google-analytics.com/ga.js';
+ var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(ga, s);
+ })();
+ </script>
+ </head>
+ <body>
+ <!-- magical breadcrumbs -->
+ <div class="topnav">
+ <ul class="breadcrumb">
+ <li>
+ <div class="dropdown">
+ <a data-toggle="dropdown" href="#">Apache Software Foundation <span class="caret"></span></a>
+ <ul class="dropdown-menu" role="menu" aria-labelledby="dLabel">
+ <li><a href="http://www.apache.org">Apache Homepage</a></li>
+ <li><a href="http://www.apache.org/licenses/">License</a></li>
+ <li><a href="http://www.apache.org/foundation/sponsorship.html">Sponsorship</a></li>
+ <li><a href="http://www.apache.org/foundation/thanks.html">Thanks</a></li>
+ <li><a href="http://www.apache.org/security/">Security</a></li>
+ </ul>
+ </div>
+ </li>
+ <li><a href="http://mesos.apache.org">Apache Mesos</a></li>
+
+
+ <li><a href="/documentation
+/">Documentation
+</a></li>
+
+
+ </ul><!-- /breadcrumb -->
+ </div>
+
+ <!-- navbar excitement -->
+ <div class="navbar navbar-static-top" role="navigation">
+ <div class="navbar-inner">
+ <div class="container">
+ <a href="/" class="logo"><img src="/assets/img/mesos_logo.png" alt="Apache Mesos logo" /></a>
+ <div class="nav-collapse">
+ <ul class="nav nav-pills navbar-right">
+ <li><a href="/gettingstarted/">Getting Started</a></li>
+ <li><a href="/documentation/latest/">Documentation</a></li>
+ <li><a href="/downloads/">Downloads</a></li>
+ <li><a href="/community/">Community</a></li>
+ </ul>
+ </div>
+ </div>
+ </div>
+ </div><!-- /.navbar -->
+
+ <div class="container">
+
+ <div class="row-fluid">
+ <div class="col-md-4">
+ <h4>If you're new to Mesos</h4>
+ <p>See the <a href="/gettingstarted/">getting started</a> page for more information about downloading, building, and deploying Mesos.</p>
+
+ <h4>If you'd like to get involved or you're looking for support</h4>
+ <p>See our <a href="/community/">community</a> page for more details.</p>
+ </div>
+ <div class="col-md-8">
+ <h1>Effective Code Reviewing</h1>
+
+<h3>Preparing for Review</h3>
+
+<p>We’ve found that clear, clean, small, independent, incremental changes are much
+easier to review thoroughly, and much easier to get committed. Here are some tips
+to consider before sending review requests:</p>
+
+<ol>
+<li><strong>Use up front discussion</strong>: Surprising a reviewer is likely to lead to
+wasted effort if the overall design needs changing. Try to get a reviewer
+on the same page before sending them a review request.</li>
+<li><strong>Think about how to break up your patch</strong>: Did you make any changes
+that could could be reviewed in independent patches? We use
+<code>support/post-reviews.py</code>, which makes it easy to create “chains” of
+reviews based on commits. Become familiar with interactive rebasing
+(<code>git rebase -i</code>) to split up commits.</li>
+<li><strong>Provide context for the change</strong>: Make the motivations for the
+change clear in the review request, so the reviewer is not left
+guessing. It is highly recommended to attach a JIRA issue with your
+review for additional context.</li>
+<li><strong>Follow the <a href="http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/">style guide</a>
+and the style of code around you</strong>.</li>
+<li><strong>Do a self-review of your changes before publishing</strong>: Approach it
+from the perspective of a reviewer with no context. Is it easy to figure
+out the motivations for the change? Does it fit in with the rest of the
+code style? Are there unnecessary changes that crept in? Did you do any
+testing of the change?</li>
+</ol>
+
+
+<h3>Reviewing</h3>
+
+<ol>
+<li><strong>Do high level reviewing before low-level reviewing</strong>: Consider the
+big picture before you get into any low-level details in the code.
+For example, do you understand what motivated the change? Why the
+author chose the particular solution? Does it “feel” more complicated
+than necessary? If so, invest the time to consider if there are
+simpler approaches to the problem, or whether the scope of the work
+can be reduced. These should all be addressed before doing a
+line-by-line thorough code review, as the latter is subject to a
+lot of review back-and-forth in the face of changes to the overall
+approach.</li>
+<li><strong>Be patient, thoughtful, and respectful</strong>: when providing (a) feedback
+on reviews and (b) commenting on feedback. Practice
+<a href="http://blog.codinghorror.com/the-ten-commandments-of-egoless-programming/">ego-less programming</a>,
+this is not meant to be a sparring match! A prerequisite to being a good
+reviewee is acknowledging that ‘writing is hard’! The reviewee should give
+the reviewer the benefit of the doubt that the reviewer has a real concern
+even if they can’t articulate it well. And the reviewee should be prepared
+to anticipate all of the reviewers concerns and be thoughtful about why
+they decided to do something a particular way (especially if it deviates
+from a standard in the codebase). On the flip side, the reviewer should
+not use the fact that ‘writing is hard’ as a crutch for giving hasty
+feedback. The reviewer should invest time and energy trying to explain
+their concerns clearly and carefully. It’s a two-way street!</li>
+<li><strong>Resolve issues before committing</strong>: we tend to give a ‘ship it’ even when
+we think there are some issues that need correcting. Sometimes a particular
+issue will require more discussion and sometimes we take that discussion to
+IM or IRC or emails to expedite the process. It’s important, however, that
+we publicly capture any collaborations/discussions that happened in those
+means. Making the discussion public also helps involve others that would
+have liked to get involved but weren’t invited.
+
+<ol type="a">
+<li>If the reviewer and reviewee are having problems resolving a particular
+“confrontational” issue then both parties should consider communicating
+directly or asking another reviewer to participate. <strong>We’re all here to
+build the highest quality code possible, and we should leverage one
+another to do so.</strong></li>
+<li>When an issue is “Dropped” by the reviewee, the expectation is that there
+must be a reply to the issue indicating why it was dropped. A silent “Drop”
+is discouraged.</li>
+<li>If an issue is marked as “Resolved”, the expectation is that the diff has
+been updated in accordance (more or less) with the reviewer’s comment. If
+there are significant changes, a reply to the issue with a comment is
+greatly appreciated.</li>
+</ol>
+</li>
+<li><strong>Be explicit about asking for more feedback</strong>: feel free to update reviews
+as often as you like but recognize that in many cases it’s ambiguous for
+reviewers when a review is back into a “ready” state so the reviewee should
+feel free to ping the reviewers via email when they’re ready.</li>
+<li><strong>For reviewees, wait for a ‘ship it’</strong>: We often use the original developer
+or current “custodian” of some particular code to be the reviewer and add others
+to get more feedback or as an FYI. Feel free to explicitly call out that you’re
+adding some people just as an FYI in the ‘Description’. It’s worth mentioning
+that we often reach out directly to one another about a particular change/feature
+before we even start coding. A little context goes a long way to streamlining the
+review process.</li>
+<li><strong>For reviewers, be thorough when giving a ‘ship it’</strong>: understand that
+reviewing requires a substantial investment of time and focus:
+
+<ol type="a">
+<li>You are expected to understand and support code that you’re giving a ‘ship it’ to.</li>
+<li>You are expected to be accountable for the quality of the code you’ve given a ‘ship it’ to.</li>
+</ol>
+</li>
+</ol>
+
+
+ </div>
+</div>
+
+
+ <hr>
+
+ <!-- footer -->
+ <div class="footer">
+ <p>© 2012-2014 <a href="http://apache.org">The Apache Software Foundation</a>.
+ Apache Mesos, the Apache feather logo, and the Apache Mesos project logo are trademarks of The Apache Software Foundation.<p>
+ </div><!-- /footer -->
+
+ </div> <!-- /container -->
+
+ <!-- JS -->
+ <script src="//code.jquery.com/jquery-1.11.0.min.js" type="text/javascript"></script>
+ <script src="//netdna.bootstrapcdn.com/bootstrap/3.1.1/js/bootstrap.min.js" type="text/javascript"></script>
+ </body>
+</html>
\ No newline at end of file
Modified: mesos/site/publish/documentation/latest/index.html
URL: http://svn.apache.org/viewvc/mesos/site/publish/documentation/latest/index.html?rev=1675732&r1=1675731&r2=1675732&view=diff
==============================================================================
--- mesos/site/publish/documentation/latest/index.html (original)
+++ mesos/site/publish/documentation/latest/index.html Thu Apr 23 23:14:45 2015
@@ -138,7 +138,8 @@
<ul>
<li><a href="/documentation/latest/reporting-a-bug/">Reporting an Issue, Improvement, or Feature</a> for getting started with JIRA.</li>
<li><a href="/documentation/latest/submitting-a-patch/">Submitting a Patch</a> for getting started with ReviewBoard, and our tooling around it.</li>
-<li><a href="/documentation/latest/committers-guide/">Committer’s Guide</a> a guiding document for etiquette in reviews and commits.</li>
+<li><a href="/documentation/latest/committers-guide/">Effective Code Reviewing</a> guidelines, tips, and learnings for how to do effective code reviews.</li>
+<li><a href="/documentation/latest/committing/">Committing</a> guidelines for committing changes.</li>
<li><a href="/documentation/latest/mesos-c++-style-guide/">C++ Style Guide</a></li>
<li><a href="/documentation/latest/mesos-roadmap/">Development Roadmap</a></li>
<li><a href="/documentation/latest/release-guide/">Release Guide</a></li>
Modified: mesos/site/publish/sitemap.xml
URL: http://svn.apache.org/viewvc/mesos/site/publish/sitemap.xml?rev=1675732&r1=1675731&r2=1675732&view=diff
==============================================================================
--- mesos/site/publish/sitemap.xml (original)
+++ mesos/site/publish/sitemap.xml Thu Apr 23 23:14:45 2015
@@ -14653,7 +14653,7 @@
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
<url>
- <loc>http://mesos.apache.org/documentation/latest/committers-guide/</loc>
+ <loc>http://mesos.apache.org/documentation/latest/committing/</loc>
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
<url>
@@ -14673,6 +14673,10 @@
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
<url>
+ <loc>http://mesos.apache.org/documentation/latest/effective-code-reviewing/</loc>
+ <lastmod>2015-04-23T00:00:00-07:00</lastmod>
+ </url>
+ <url>
<loc>http://mesos.apache.org/documentation/latest/external-containerizer/</loc>
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
@@ -14805,7 +14809,7 @@
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
<url>
- <loc>http://mesos.apache.org/documentation/committers-guide/</loc>
+ <loc>http://mesos.apache.org/documentation/committing/</loc>
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
<url>
@@ -14825,6 +14829,10 @@
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
<url>
+ <loc>http://mesos.apache.org/documentation/effective-code-reviewing/</loc>
+ <lastmod>2015-04-23T00:00:00-07:00</lastmod>
+ </url>
+ <url>
<loc>http://mesos.apache.org/documentation/external-containerizer/</loc>
<lastmod>2015-04-23T00:00:00-07:00</lastmod>
</url>
Modified: mesos/site/source/documentation/latest.html.md
URL: http://svn.apache.org/viewvc/mesos/site/source/documentation/latest.html.md?rev=1675732&r1=1675731&r2=1675732&view=diff
==============================================================================
--- mesos/site/source/documentation/latest.html.md (original)
+++ mesos/site/source/documentation/latest.html.md Thu Apr 23 23:14:45 2015
@@ -43,7 +43,8 @@ layout: documentation
* [Reporting an Issue, Improvement, or Feature](/documentation/latest/reporting-a-bug/) for getting started with JIRA.
* [Submitting a Patch](/documentation/latest/submitting-a-patch/) for getting started with ReviewBoard, and our tooling around it.
-* [Committer's Guide](/documentation/latest/committers-guide/) a guiding document for etiquette in reviews and commits.
+* [Effective Code Reviewing](/documentation/latest/committers-guide/) guidelines, tips, and learnings for how to do effective code reviews.
+* [Committing](/documentation/latest/committing/) guidelines for committing changes.
* [C++ Style Guide](/documentation/latest/mesos-c++-style-guide/)
* [Development Roadmap](/documentation/latest/mesos-roadmap/)
* [Release Guide](/documentation/latest/release-guide/)
Added: mesos/site/source/documentation/latest/committing.md
URL: http://svn.apache.org/viewvc/mesos/site/source/documentation/latest/committing.md?rev=1675732&view=auto
==============================================================================
--- mesos/site/source/documentation/latest/committing.md (added)
+++ mesos/site/source/documentation/latest/committing.md Thu Apr 23 23:14:45 2015
@@ -0,0 +1,33 @@
+---
+layout: documentation
+---
+
+# Committing
+
+Only committers have the ability to commit your change, so make sure you
+work with one during code review to get your changes committed.
+
+If you are a committer, here are some guidelines for committing changes:
+
+1. **Follow the format of commit messages**: we currently do not leverage
+ any tooling to enforce the format of messages, so:
+ a. Be clear and explicit in the commit message.
+ b. Include the link to the review (this will be done automatically if
+ using `support/post-reviews.py` for your own changes and
+ `support/apply-review.sh` for committing changes from others).
+ **Be sure to clean up the commit message when pulling in changes
+ from others.**
+ c. Use 72 character columns. Note that we don't always have a 50
+ character or less summary because that restriction tends to cause
+ people to write poorly.
+2. **Keep the 'Author' intact**: `support/apply-review.sh` will handle
+ this for you, but be careful when rebasing or ammending.
+3. **Never ever commit a merge**: always rebase instead, as appropriate.
+ Likewise, never 'force push'.
+4. **Don't break the build**: we support Linux and Mac OS X, however not
+ all configurations are being tested in Jenkins, so be aware of that.
+ Also, pay attention to the Jenkins review bot if it flags a review as
+ breaking the build. Note that if you do break the build, the fixes are
+ often small and inconsequential so don't worry about going through a
+ review cycle for that, just fix things (but don't take that as a license
+ to "wait until the build fails to fix things").
\ No newline at end of file
Added: mesos/site/source/documentation/latest/effective-code-reviewing.md
URL: http://svn.apache.org/viewvc/mesos/site/source/documentation/latest/effective-code-reviewing.md?rev=1675732&view=auto
==============================================================================
--- mesos/site/source/documentation/latest/effective-code-reviewing.md (added)
+++ mesos/site/source/documentation/latest/effective-code-reviewing.md Thu Apr 23 23:14:45 2015
@@ -0,0 +1,91 @@
+---
+layout: documentation
+---
+
+# Effective Code Reviewing
+
+### Preparing for Review
+
+We've found that clear, clean, small, independent, incremental changes are much
+easier to review thoroughly, and much easier to get committed. Here are some tips
+to consider before sending review requests:
+
+1. **Use up front discussion**: Surprising a reviewer is likely to lead to
+ wasted effort if the overall design needs changing. Try to get a reviewer
+ on the same page before sending them a review request.
+2. **Think about how to break up your patch**: Did you make any changes
+ that could could be reviewed in independent patches? We use
+ `support/post-reviews.py`, which makes it easy to create "chains" of
+ reviews based on commits. Become familiar with interactive rebasing
+ (`git rebase -i`) to split up commits.
+3. **Provide context for the change**: Make the motivations for the
+ change clear in the review request, so the reviewer is not left
+ guessing. It is highly recommended to attach a JIRA issue with your
+ review for additional context.
+4. **Follow the [style guide](http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/)
+ and the style of code around you**.
+5. **Do a self-review of your changes before publishing**: Approach it
+ from the perspective of a reviewer with no context. Is it easy to figure
+ out the motivations for the change? Does it fit in with the rest of the
+ code style? Are there unnecessary changes that crept in? Did you do any
+ testing of the change?
+
+### Reviewing
+
+1. **Do high level reviewing before low-level reviewing**: Consider the
+ big picture before you get into any low-level details in the code.
+ For example, do you understand what motivated the change? Why the
+ author chose the particular solution? Does it "feel" more complicated
+ than necessary? If so, invest the time to consider if there are
+ simpler approaches to the problem, or whether the scope of the work
+ can be reduced. These should all be addressed before doing a
+ line-by-line thorough code review, as the latter is subject to a
+ lot of review back-and-forth in the face of changes to the overall
+ approach.
+2. **Be patient, thoughtful, and respectful**: when providing (a) feedback
+ on reviews and (b) commenting on feedback. Practice
+ [ego-less programming](http://blog.codinghorror.com/the-ten-commandments-of-egoless-programming/),
+ this is not meant to be a sparring match! A prerequisite to being a good
+ reviewee is acknowledging that 'writing is hard'! The reviewee should give
+ the reviewer the benefit of the doubt that the reviewer has a real concern
+ even if they can't articulate it well. And the reviewee should be prepared
+ to anticipate all of the reviewers concerns and be thoughtful about why
+ they decided to do something a particular way (especially if it deviates
+ from a standard in the codebase). On the flip side, the reviewer should
+ not use the fact that 'writing is hard' as a crutch for giving hasty
+ feedback. The reviewer should invest time and energy trying to explain
+ their concerns clearly and carefully. It's a two-way street!
+3. **Resolve issues before committing**: we tend to give a 'ship it' even when
+ we think there are some issues that need correcting. Sometimes a particular
+ issue will require more discussion and sometimes we take that discussion to
+ IM or IRC or emails to expedite the process. It's important, however, that
+ we publicly capture any collaborations/discussions that happened in those
+ means. Making the discussion public also helps involve others that would
+ have liked to get involved but weren't invited.
+ a. If the reviewer and reviewee are having problems resolving a particular
+ "confrontational" issue then both parties should consider communicating
+ directly or asking another reviewer to participate. **We're all here to
+ build the highest quality code possible, and we should leverage one
+ another to do so.**
+ b. When an issue is "Dropped" by the reviewee, the expectation is that there
+ must be a reply to the issue indicating why it was dropped. A silent "Drop"
+ is discouraged.
+ c. If an issue is marked as "Resolved", the expectation is that the diff has
+ been updated in accordance (more or less) with the reviewer's comment. If
+ there are significant changes, a reply to the issue with a comment is
+ greatly appreciated.
+4. **Be explicit about asking for more feedback**: feel free to update reviews
+as often as you like but recognize that in many cases it's ambiguous for
+reviewers when a review is back into a "ready" state so the reviewee should
+feel free to ping the reviewers via email when they're ready.
+5. **For reviewees, wait for a 'ship it'**: We often use the original developer
+or current "custodian" of some particular code to be the reviewer and add others
+to get more feedback or as an FYI. Feel free to explicitly call out that you're
+adding some people just as an FYI in the 'Description'. It's worth mentioning
+that we often reach out directly to one another about a particular change/feature
+before we even start coding. A little context goes a long way to streamlining the
+review process.
+6. **For reviewers, be thorough when giving a 'ship it'**: understand that
+reviewing requires a substantial investment of time and focus:
+ a. You are expected to understand and support code that you're giving a 'ship it' to.
+ b. You are expected to be accountable for the quality of the code you've given a 'ship it' to.
\ No newline at end of file