This is an automated email from the ASF dual-hosted git repository. fhueske pushed a commit to branch asf-site in repository https://gitbox.apache.org/repos/asf/flink-web.git
commit ca75c24ac7d37fc7ada451e2d381fa08c8838d8f Author: Fabian Hueske <[email protected]> AuthorDate: Thu Oct 18 10:02:51 2018 +0200 Rebuild website --- content/how-to-contribute.html | 9 ++ content/reviewing-prs.html | 304 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 313 insertions(+) diff --git a/content/how-to-contribute.html b/content/how-to-contribute.html index 38486ef..216e412 100644 --- a/content/how-to-contribute.html +++ b/content/how-to-contribute.html @@ -149,6 +149,7 @@ <li><a href="#file-a-bug-report" id="markdown-toc-file-a-bug-report">File a bug report</a></li> <li><a href="#propose-an-improvement-or-a-new-feature" id="markdown-toc-propose-an-improvement-or-a-new-feature">Propose an improvement or a new feature</a></li> <li><a href="#help-others-and-join-the-discussions" id="markdown-toc-help-others-and-join-the-discussions">Help others and join the discussions</a></li> + <li><a href="#review-a-code-contribution" id="markdown-toc-review-a-code-contribution">Review a code contribution</a></li> <li><a href="#test-a-release-candidate" id="markdown-toc-test-a-release-candidate">Test a release candidate</a></li> <li><a href="#contribute-code" id="markdown-toc-contribute-code">Contribute code</a></li> <li><a href="#contribute-documentation" id="markdown-toc-contribute-documentation">Contribute documentation</a></li> @@ -205,6 +206,14 @@ discussions on existing proposals.</p> <hr /> +<h2 id="review-a-code-contribution">Review a code contribution</h2> + +<p>The Apache Flink project receives many code contributions as <a href="https://github.com/apache/flink/pulls";>Github pull requests</a>. A great way to contribute to the Flink community is to help review pull requests.</p> + +<p><strong>Please read the <a href="/reviewing-prs.html">Review Guide</a> if you want to help review pull requests.</strong></p> + +<hr /> + <h2 id="test-a-release-candidate">Test a release candidate</h2> <p>Apache Flink is continuously improved by its active community. Every few weeks, we release a new version of Apache Flink with bug fixes, improvements, and new features. The process of releasing a new version consists of the following steps:</p> diff --git a/content/reviewing-prs.html b/content/reviewing-prs.html new file mode 100644 index 0000000..6ad58af --- /dev/null +++ b/content/reviewing-prs.html @@ -0,0 +1,304 @@ +<!DOCTYPE html> +<html lang="en"> + <head> + <meta charset="utf-8"> + <meta http-equiv="X-UA-Compatible" content="IE=edge"> + <meta name="viewport" content="width=device-width, initial-scale=1"> + <!-- The above 3 meta tags *must* come first in the head; any other head content must come *after* these tags --> + <title>Apache Flink: How to Review a Pull Request</title> + <link rel="shortcut icon" href="/favicon.ico" type="image/x-icon"> + <link rel="icon" href="/favicon.ico" type="image/x-icon"> + + <!-- Bootstrap --> + <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.4/css/bootstrap.min.css";> + <link rel="stylesheet" href="/css/flink.css"> + <link rel="stylesheet" href="/css/syntax.css"> + + <!-- Blog RSS feed --> + <link href="/blog/feed.xml" rel="alternate" type="application/rss+xml" title="Apache Flink Blog: RSS feed" /> + + <!-- jQuery (necessary for Bootstrap's JavaScript plugins) --> + <!-- We need to load Jquery in the header for custom google analytics event tracking--> + <script src="/js/jquery.min.js"></script> + + <!-- HTML5 shim and Respond.js for IE8 support of HTML5 elements and media queries --> + <!-- WARNING: Respond.js doesn't work if you view the page via file:// --> + <!--[if lt IE 9]> + <script src="https://oss.maxcdn.com/html5shiv/3.7.2/html5shiv.min.js";></script> + <script src="https://oss.maxcdn.com/respond/1.4.2/respond.min.js";></script> + <![endif]--> + </head> + <body> + + + <!-- Main content. --> + <div class="container"> + <div class="row"> + + + <div id="sidebar" class="col-sm-3"> + <!-- Top navbar. --> + <nav class="navbar navbar-default"> + <!-- The logo. --> + <div class="navbar-header"> + <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#bs-example-navbar-collapse-1"> + <span class="icon-bar"></span> + <span class="icon-bar"></span> + <span class="icon-bar"></span> + </button> + <div class="navbar-logo"> + <a href="/"> + <img alt="Apache Flink" src="/img/flink-header-logo.svg" width="147px" height="73px"> + </a> + </div> + </div><!-- /.navbar-header --> + + <!-- The navigation links. --> + <div class="collapse navbar-collapse" id="bs-example-navbar-collapse-1"> + <ul class="nav navbar-nav navbar-main"> + + <!-- First menu section explains visitors what Flink is --> + + <!-- What is Stream Processing? --> + <!-- + <li><a href="/streamprocessing1.html">What is Stream Processing?</a></li> + --> + + <!-- What is Flink? --> + <li><a href="/flink-architecture.html">What is Apache Flink?</a></li> + + <!-- Use cases --> + <li><a href="/usecases.html">Use Cases</a></li> + + <!-- Powered by --> + <li><a href="/poweredby.html">Powered By</a></li> + + <!-- FAQ --> + <li><a href="/faq.html">FAQ</a></li> + + + <!-- Second menu section aims to support Flink users --> + + <!-- Downloads --> + <li><a href="/downloads.html">Downloads</a></li> + + <!-- Quickstart --> + <li> + <a href="https://ci.apache.org/projects/flink/flink-docs-release-1.6/quickstart/setup_quickstart.html"; target="_blank">Tutorials <small><span class="glyphicon glyphicon-new-window"></span></small></a> + </li> + + <!-- Documentation --> + <li class="dropdown"> + <a class="dropdown-toggle" data-toggle="dropdown" href="#">Documentation<span class="caret"></span></a> + <ul class="dropdown-menu"> + <li><a href="https://ci.apache.org/projects/flink/flink-docs-release-1.6"; target="_blank">1.6 (Latest stable release) <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> + <li><a href="https://ci.apache.org/projects/flink/flink-docs-master"; target="_blank">1.7 (Snapshot) <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> + </ul> + </li> + + <!-- getting help --> + <li><a href="/gettinghelp.html">Getting Help</a></li> + + <!-- Blog --> + <li><a href="/blog/"><b>Flink Blog</b></a></li> + + + + <!-- Third menu section aim to support community and contributors --> + + <!-- Community --> + <li><a href="/community.html">Community & Project Info</a></li> + + <!-- Contribute --> + <li><a href="/how-to-contribute.html">How to Contribute</a></li> + + <!-- GitHub --> + <li> + <a href="https://github.com/apache/flink"; target="_blank">Flink on GitHub <small><span class="glyphicon glyphicon-new-window"></span></small></a> + </li> + + </ul> + + + <ul class="nav navbar-nav navbar-bottom"> + <hr /> + + <!-- Twitter --> + <li><a href="https://twitter.com/apacheflink"; target="_blank">@ApacheFlink <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> + + <!-- Visualizer --> + <li class=" hidden-md hidden-sm"><a href="/visualizer/" target="_blank">Plan Visualizer <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> + + </ul> + </div><!-- /.navbar-collapse --> + </nav> + + </div> + <div class="col-sm-9"> + <div class="row-fluid"> + <div class="col-sm-12"> + <h1>How to Review a Pull Request</h1> + + <hr /> + +<p>This guide is for all committers and contributors that want to help with reviewing code contributions. Thank you for your effort - good reviews are one the most important and crucial parts of an open source project. This guide should help the community to make reviews such that:</p> + +<ul> + <li>Contributors have a good contribution experience.</li> + <li>Our reviews are structured and check all important aspects of a contribution.</li> + <li>We make sure to keep a high code quality in Flink.</li> + <li>We avoid situations where contributors and reviewers spend a lot of time refining a contribution that gets rejected later.</li> +</ul> + +<hr /> + +<div class="page-toc"> +<ul id="markdown-toc"> + <li><a href="#review-checklist" id="markdown-toc-review-checklist">Review Checklist</a> <ul> + <li><a href="#is-the-contribution-well-described" id="markdown-toc-is-the-contribution-well-described">1. Is the Contribution Well-Described?</a></li> + <li><a href="#is-there-consensus-that-the-change-or-feature-should-go-into-flink" id="markdown-toc-is-there-consensus-that-the-change-or-feature-should-go-into-flink">2. Is There Consensus that the Change or Feature Should Go into Flink?</a></li> + <li><a href="#does-the-contribution-need-attention-from-some-specific-committers-and-is-there-time-commitment-from-these-committers" id="markdown-toc-does-the-contribution-need-attention-from-some-specific-committers-and-is-there-time-commitment-from-these-committers">3. Does the Contribution Need Attention from some Specific Committers and Is There Time Commitment from These Committers?</a></li> + <li><a href="#does-the-implementation-follow-the-right-overall-approacharchitecture" id="markdown-toc-does-the-implementation-follow-the-right-overall-approacharchitecture">4. Does the Implementation Follow the Right Overall Approach/Architecture?</a></li> + <li><a href="#is-the-overall-code-quality-good-meeting-standard-we-want-to-maintain-in-flink" id="markdown-toc-is-the-overall-code-quality-good-meeting-standard-we-want-to-maintain-in-flink">5. Is the Overall Code Quality Good, Meeting Standard we Want to Maintain in Flink?</a></li> + </ul> + </li> + <li><a href="#review-template" id="markdown-toc-review-template">Review Template</a></li> +</ul> + +</div> + +<h2 id="review-checklist">Review Checklist</h2> + +<p>Every review needs to check the following five aspects. We encourage to check these aspects in order, to avoid spending time on detailed code quality reviews when there is no consensus yet whether a feature or change should actually be added.</p> + +<h3 id="is-the-contribution-well-described">1. Is the Contribution Well-Described?</h3> + +<p>Check whether the contribution is sufficiently well-described to support a good review. Trivial changes and fixes do not need a long description. Any pull request that changes functionality or behavior needs to describe the big picture of these changes, so that reviews know what to look for (and don’t have to dig through the code to hopefully understand what the change does).</p> + +<p>Changes that require longer descriptions are ideally based on a prior design discussion in the mailing list or in Jira and can simply link to there or copy the description from there.</p> + +<p><strong>A contribution is well-described if the following questions 2, 3, and 4 can be answered without looking at the code.</strong></p> + +<hr /> + +<h3 id="is-there-consensus-that-the-change-or-feature-should-go-into-flink">2. Is There Consensus that the Change or Feature Should Go into Flink?</h3> + +<p>For bug fixes, this needs to be checked only in case it requires bigger changes or might break existing programs and setups.</p> + +<p>Ideally, this question can be directly answered from a Jira issue or a dev-list discussion, except in cases of bug fixes and small lightweight additions/extensions. In that case, this question can be immediately marked as resolved. For pull requests that are created without prior consensus, this question needs to be answered as part of the review.</p> + +<p>The decision whether the change should go into Flink needs to take the following aspects into consideration:</p> + +<ul> + <li>Does the contribution alter the behavior of features or components in a way that it may break previous users’ programs and setups? If yes, there needs to be a discussion and agreement that this change is desirable.</li> + <li>Does the contribution conceptually fit well into Flink? Is it too much of a special case such that it makes things more complicated for the common case, or bloats the abstractions / APIs?</li> + <li>Does the feature fit well into Flink’s architecture? Will it scale and keep Flink flexible for the future, or will the feature restrict Flink in the future?</li> + <li>Is the feature a significant new addition (rather than an improvement to an existing part)? If yes, will the Flink community commit to maintaining this feature?</li> + <li>Does the feature produce added value for Flink users or developers? Or does it introduce the risk of regression without adding relevant user or developer benefit?</li> + <li>Could the contribution live in another repository, e.g., <a href="https://bahir.apache.org";>Apache Bahir</a> or another external repository?</li> +</ul> + +<p>All of these questions should be answerable from the description/discussion in Jira and Pull Request, without looking at the code.</p> + +<p><strong>A feature, improvement, or bug fix is approved once one committer accepts it and no committer disagrees (lazy consensus).</strong></p> + +<p>In case of diverging opinions, the discussion should be moved to the respective Jira issue or to the dev mailing list and continued until consensus is reached. If the change is proposed by a committer, it is best-practice to seek the approval of another committer.</p> + +<hr /> + +<h3 id="does-the-contribution-need-attention-from-some-specific-committers-and-is-there-time-commitment-from-these-committers">3. Does the Contribution Need Attention from some Specific Committers and Is There Time Commitment from These Committers?</h3> + +<p>Some changes require attention and approval from specific committers. For example, changes in parts that are either very performance sensitive, or have a critical impact on distributed coordination and fault tolerance need input by a committer that is deeply familiar with the component.</p> + +<p>As a rule of thumb, special attention is required when the Pull Request description answers one of the questions in the template section “Does this pull request potentially affect one of the following parts” with ‘yes’.</p> + +<p>This question can be answered with</p> + +<ul> + <li><em>Does not need specific attention</em></li> + <li><em>Needs specific attention for X (X can be for example checkpointing, jobmanager, etc.).</em></li> + <li><em>Has specific attention for X by @commiterA, @contributorB</em></li> +</ul> + +<p><strong>If the pull request needs specific attention, one of the tagged committers/contributors should give the final approval.</strong></p> + +<hr /> + +<h3 id="does-the-implementation-follow-the-right-overall-approacharchitecture">4. Does the Implementation Follow the Right Overall Approach/Architecture?</h3> + +<p>Is this the best approach to implement the fix or feature, or are there other approaches that would be easier, more robust, or more maintainable? +This question should be answerable from the Pull Request description (or the linked Jira) as much as possible.</p> + +<p>We recommend to check this before diving into the details of commenting on individual parts of the change.</p> + +<hr /> + +<h3 id="is-the-overall-code-quality-good-meeting-standard-we-want-to-maintain-in-flink">5. Is the Overall Code Quality Good, Meeting Standard we Want to Maintain in Flink?</h3> + +<p>This is the detailed code review of the actual changes, covering:</p> + +<ul> + <li>Are the changes doing what is described in the design document or PR description?</li> + <li>Does the code follow the right software engineering practices? Is the code correct, robust, maintainable, testable?</li> + <li>Are the changes performance aware, when changing a performance sensitive part?</li> + <li>Are the changes sufficiently covered by tests?</li> + <li>Are the tests executing fast, i.e., are heavy-weight integration tests only used when necessary?</li> + <li>Does the code format follow Flink’s checkstyle pattern?</li> + <li>Does the code avoid to introduce additional compiler warnings?</li> +</ul> + +<p>Some code style guidelines can be found in the <a href="/contribute-code.html#code-style">Flink Code Style Page</a></p> + +<h2 id="review-template">Review Template</h2> + +<p>The “Review Progress” section is posted at the end of the description of every new pull request.</p> + +<div class="highlight"><pre><code>### Review Progress <!-- NOTE: DO NOT REMOVE THIS SECTION! --> + +* [ ] 1. The contribution is well-described. +* [ ] 2. There is consensus that the contribution should go into to Flink. +* [ ] 3. [Does not need specific attention | Needs specific attention for X | Has attention for X by Y] +* [ ] 4. The architectural approach is sound. +* [ ] 5. Overall code quality is good. + +Please see the [Pull Request Review Guide](https://flink.apache.org/reviewing-prs.html) if you have questions about the review process. +</code></pre></div> + +<p>Reviewers should tick off the boxes (in order) to indicate the progress of the review.</p> + + + </div> +</div> + + </div> + </div> + + <hr /> + + <div class="row"> + <div class="footer text-center col-sm-12"> + <p>Copyright © 2014-2017 <a href="http://apache.org";>The Apache Software Foundation</a>. All Rights Reserved.</p> + <p>Apache Flink, Flink®, Apache®, the squirrel logo, and the Apache feather logo are either registered trademarks or trademarks of The Apache Software Foundation.</p> + <p><a href="/privacy-policy.html">Privacy Policy</a> · <a href="/blog/feed.xml">RSS feed</a></p> + </div> + </div> + </div><!-- /.container --> + + <!-- Include all compiled plugins (below), or include individual files as needed --> + <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.4/js/bootstrap.min.js";></script> + <script src="/js/codetabs.js"></script> + <script src="/js/stickysidebar.js"></script> + + + <!-- Google Analytics --> + <script> + (function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ + (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), + m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) + })(window,document,'script','//www.google-analytics.com/analytics.js','ga'); + + ga('create', 'UA-52545728-1', 'auto'); + ga('send', 'pageview'); + </script> + </body> +</html>
