| <!DOCTYPE html> |
| <html> |
| |
| |
| |
| |
| <head> |
| <meta charset="utf-8"> |
| <meta http-equiv="X-UA-Compatible" content="IE=edge"> |
| <meta name="viewport" content="width=device-width, initial-scale=1"> |
| <meta name="description" content="The Apache Cassandra database is the right choice when you need scalability and high availability without compromising performance. Linear scalability and proven fault-tolerance on commodity hardware or cloud infrastructure make it the perfect platform for mission-critical data. Cassandra's support for replicating across multiple datacenters is best-in-class, providing lower latency for your users and the peace of mind of knowing that you can survive regional outages. |
| "> |
| <meta name="keywords" content="cassandra, apache, apache cassandra, distributed storage, key value store, scalability, bigtable, dynamo" /> |
| <meta name="robots" content="index,follow" /> |
| <meta name="language" content="en" /> |
| |
| <title>Documentation</title> |
| |
| <link rel="canonical" href="http://cassandra.apache.org/doc/3.10/development/how_to_review.html"> |
| |
| <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css" integrity="sha384-1q8mTJOASx8j1Au+a5WDVnPi2lkFfwwEAa8hDDdjZlpLegxhjVME1fgjWPGmkzs7" crossorigin="anonymous"> |
| <link rel="stylesheet" href="./../../../css/style.css"> |
| |
| <link rel="stylesheet" href="./../../../css/sphinx.css"> |
| |
| |
| <link rel="top" title="Apache Cassandra Documentation v3.10" href="../index.html"/> <link rel="up" title="Cassandra Development" href="index.html"/> <link rel="next" title="How-to Commit" href="how_to_commit.html"/> <link rel="prev" title="Code Style" href="code_style.html"/> |
| </head> |
| |
| <body> |
| <!-- breadcrumbs --> |
| <div class="topnav"> |
| <div class="container breadcrumb-container"> |
| <ul class="breadcrumb"> |
| <li> |
| <div class="dropdown"> |
| <img class="asf-logo" src="./../../../img/asf_feather.png" /> |
| <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="./../../../">Apache Cassandra</a></li> |
| |
| |
| |
| |
| <li><a href="./../../../doc">Documentation</a></li> |
| |
| |
| |
| |
| <li><a href="./">Cassandra Development</a></li> |
| |
| |
| |
| <li>Review Checklist</li> |
| |
| </ul> |
| </div> |
| |
| <!-- navbar --> |
| <nav class="navbar navbar-default navbar-static-top" role="navigation"> |
| <div class="container"> |
| <div class="navbar-header"> |
| <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#cassandra-menu" aria-expanded="false"> |
| <span class="sr-only">Toggle navigation</span> |
| <span class="icon-bar"></span> |
| <span class="icon-bar"></span> |
| <span class="icon-bar"></span> |
| </button> |
| <a class="navbar-brand" href="./../../../"><img src="./../../../img/cassandra_logo.png" alt="Apache Cassandra logo" /></a> |
| </div><!-- /.navbar-header --> |
| |
| <div id="cassandra-menu" class="collapse navbar-collapse"> |
| <ul class="nav navbar-nav navbar-right"> |
| <li><a href="./../../../">Home</a></li> |
| <li><a href="./../../../download/">Download</a></li> |
| <li><a href="./../../../doc/">Documentation</a></li> |
| <li><a href="./../../../community/">Community</a></li> |
| </ul> |
| </div><!-- /#cassandra-menu --> |
| |
| |
| </div> |
| </nav><!-- /.navbar --> |
| </div><!-- /.topnav --> |
| |
| <div class="container-fluid"> |
| <div class="row"> |
| <div class="col-md-2"> |
| <div class="doc-navigation"> |
| <div class="doc-menu" role="navigation"> |
| <div class="navbar-header"> |
| <button type="button" class="pull-left navbar-toggle" data-toggle="collapse" data-target=".sidebar-navbar-collapse"> |
| <span class="sr-only">Toggle navigation</span> |
| <span class="icon-bar"></span> |
| <span class="icon-bar"></span> |
| <span class="icon-bar"></span> |
| </button> |
| </div> |
| <div class="navbar-collapse collapse sidebar-navbar-collapse"> |
| <form id="doc-search-form" class="navbar-form" action="../search.html" method="get" role="search"> |
| <div class="form-group"> |
| <input type="text" size="30" class="form-control input-sm" name="q" placeholder="Search docs"> |
| <input type="hidden" name="check_keywords" value="yes" /> |
| <input type="hidden" name="area" value="default" /> |
| </div> |
| </form> |
| |
| |
| |
| <ul class="current"> |
| <li class="toctree-l1"><a class="reference internal" href="../getting_started/index.html">Getting Started</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../architecture/index.html">Architecture</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../data_modeling/index.html">Data Modeling</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../cql/index.html">The Cassandra Query Language (CQL)</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../configuration/index.html">Configuring Cassandra</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../operating/index.html">Operating Cassandra</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../tools/index.html">Cassandra Tools</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../troubleshooting/index.html">Troubleshooting</a></li> |
| <li class="toctree-l1 current"><a class="reference internal" href="index.html">Cassandra Development</a><ul class="current"> |
| <li class="toctree-l2"><a class="reference internal" href="ide.html">Building and IDE Integration</a></li> |
| <li class="toctree-l2"><a class="reference internal" href="testing.html">Testing</a></li> |
| <li class="toctree-l2"><a class="reference internal" href="patches.html">Contributing Code Changes</a></li> |
| <li class="toctree-l2"><a class="reference internal" href="code_style.html">Code Style</a></li> |
| <li class="toctree-l2 current"><a class="current reference internal" href="#">Review Checklist</a></li> |
| <li class="toctree-l2"><a class="reference internal" href="how_to_commit.html">How-to Commit</a></li> |
| </ul> |
| </li> |
| <li class="toctree-l1"><a class="reference internal" href="../faq/index.html">Frequently Asked Questions</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../bugs.html">Reporting Bugs and Contributing</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../contactus.html">Contact us</a></li> |
| </ul> |
| |
| |
| |
| </div><!--/.nav-collapse --> |
| </div> |
| </div> |
| </div> |
| <div class="col-md-8"> |
| <div class="content doc-content"> |
| <div class="container"> |
| |
| <div class="section" id="review-checklist"> |
| <h1>Review Checklist<a class="headerlink" href="#review-checklist" title="Permalink to this headline">¶</a></h1> |
| <p>When reviewing tickets in Apache JIRA, the following items should be covered as part of the review process:</p> |
| <p><strong>General</strong></p> |
| <blockquote> |
| <div><ul class="simple"> |
| <li>Does it conform to the <a class="reference internal" href="code_style.html"><span class="doc">Code Style</span></a> guidelines?</li> |
| <li>Is there any redundant or duplicate code?</li> |
| <li>Is the code as modular as possible?</li> |
| <li>Can any singletons be avoided?</li> |
| <li>Can any of the code be replaced with library functions?</li> |
| <li>Are units of measurement used in the code consistent, both internally and with the rest of the ecosystem?</li> |
| </ul> |
| </div></blockquote> |
| <p><strong>Error-Handling</strong></p> |
| <blockquote> |
| <div><ul class="simple"> |
| <li>Are all data inputs and outputs checked (for the correct type, length, format, and range) and encoded?</li> |
| <li>Where third-party utilities are used, are returning errors being caught?</li> |
| <li>Are invalid parameter values handled?</li> |
| <li>Are any Throwable/Exceptions passed to the JVMStabilityInspector?</li> |
| <li>Are errors well-documented? Does the error message tell the user how to proceed?</li> |
| <li>Do exceptions propagate to the appropriate level in the code?</li> |
| </ul> |
| </div></blockquote> |
| <p><strong>Documentation</strong></p> |
| <blockquote> |
| <div><ul class="simple"> |
| <li>Do comments exist and describe the intent of the code (the “why”, not the “how”)?</li> |
| <li>Are javadocs added where appropriate?</li> |
| <li>Is any unusual behavior or edge-case handling described?</li> |
| <li>Are data structures and units of measurement explained?</li> |
| <li>Is there any incomplete code? If so, should it be removed or flagged with a suitable marker like ‘TODO’?</li> |
| <li>Does the code self-document via clear naming, abstractions, and flow control?</li> |
| <li>Have NEWS.txt, the cql3 docs, and the native protocol spec been updated if needed?</li> |
| <li>Is the ticket tagged with “client-impacting” and “doc-impacting”, where appropriate?</li> |
| <li>Has lib/licences been updated for third-party libs? Are they Apache License compatible?</li> |
| <li>Is the Component on the JIRA ticket set appropriately?</li> |
| </ul> |
| </div></blockquote> |
| <p><strong>Testing</strong></p> |
| <blockquote> |
| <div><ul class="simple"> |
| <li>Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.</li> |
| <li>Do tests exist and are they comprehensive?</li> |
| <li>Do unit tests actually test that the code is performing the intended functionality?</li> |
| <li>Could any test code use common functionality (e.g. ccm, dtest, or CqlTester methods) or abstract it there for reuse?</li> |
| <li>If the code may be affected by multi-node clusters, are there dtests?</li> |
| <li>If the code may take a long time to test properly, are there CVH tests?</li> |
| <li>Is the test passing on CI for all affected branches (up to trunk, if applicable)? Are there any regressions?</li> |
| <li>If patch affects read/write path, did we test for performance regressions w/multiple workloads?</li> |
| <li>If adding a new feature, were tests added and performed confirming it meets the expected SLA/use-case requirements for the feature?</li> |
| </ul> |
| </div></blockquote> |
| <p><strong>Logging</strong></p> |
| <blockquote> |
| <div><ul class="simple"> |
| <li>Are logging statements logged at the correct level?</li> |
| <li>Are there logs in the critical path that could affect performance?</li> |
| <li>Is there any log that could be added to communicate status or troubleshoot potential problems in this feature?</li> |
| <li>Can any unnecessary logging statement be removed?</li> |
| </ul> |
| </div></blockquote> |
| </div> |
| |
| |
| |
| |
| <div class="doc-prev-next-links" role="navigation" aria-label="footer navigation"> |
| |
| <a href="how_to_commit.html" class="btn btn-default pull-right " role="button" title="How-to Commit" accesskey="n">Next <span class="glyphicon glyphicon-circle-arrow-right" aria-hidden="true"></span></a> |
| |
| |
| <a href="code_style.html" class="btn btn-default" role="button" title="Code Style" accesskey="p"><span class="glyphicon glyphicon-circle-arrow-left" aria-hidden="true"></span> Previous</a> |
| |
| </div> |
| |
| </div> |
| </div> |
| </div> |
| <div class="col-md-2"> |
| </div> |
| </div> |
| </div> |
| |
| <footer> |
| <div class="container"> |
| <div class="col-md-4 social-blk"> |
| <span class="social"> |
| <a href="https://twitter.com/cassandra" |
| class="twitter-follow-button" |
| data-show-count="false" data-size="large">Follow @cassandra</a> |
| <script>!function(d,s,id){var js,fjs=d.getElementsByTagName(s)[0],p=/^http:/.test(d.location)?'http':'https';if(!d.getElementById(id)){js=d.createElement(s);js.id=id;js.src=p+'://platform.twitter.com/widgets.js';fjs.parentNode.insertBefore(js,fjs);}}(document, 'script', 'twitter-wjs');</script> |
| <a href="https://twitter.com/intent/tweet?button_hashtag=cassandra" |
| class="twitter-hashtag-button" |
| data-size="large" |
| data-related="ApacheCassandra">Tweet #cassandra</a> |
| <script>!function(d,s,id){var js,fjs=d.getElementsByTagName(s)[0],p=/^http:/.test(d.location)?'http':'https';if(!d.getElementById(id)){js=d.createElement(s);js.id=id;js.src=p+'://platform.twitter.com/widgets.js';fjs.parentNode.insertBefore(js,fjs);}}(document, 'script', 'twitter-wjs');</script> |
| </span> |
| </div> |
| |
| <div class="col-md-8 trademark"> |
| <p>© 2016 <a href="http://apache.org">The Apache Software Foundation</a>. |
| Apache, the Apache feather logo, and Apache Cassandra are trademarks of The Apache Software Foundation. |
| <p> |
| </div> |
| </div><!-- /.container --> |
| </footer> |
| |
| <!-- Javascript. Placed here so pages load faster --> |
| <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.3/jquery.min.js"></script> |
| <script src="./../../../js/underscore-min.js"></script> |
| <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js" integrity="sha384-0mSbJDEHialfmuBBQP6A4Qrprq5OVfW37PRR3j5ELqxss1yVqOtnepnHVP9aJ7xS" crossorigin="anonymous"></script> |
| |
| |
| <script src="./../../../js/doctools.js"></script> |
| <script src="./../../../js/searchtools.js"></script> |
| |
| <script type="text/javascript"> var DOCUMENTATION_OPTIONS = { URL_ROOT: "", VERSION: "", COLLAPSE_INDEX: false, FILE_SUFFIX: ".html", HAS_SOURCE: false, SOURCELINK_SUFFIX: "" }; </script> |
| |
| <script type="text/javascript"> |
| $(function() { |
| // Stick the #nav to the top of the window |
| var nav = $('.doc-navigation'); |
| var navHomeY = nav.offset().top; |
| var isFixed = false; |
| var $w = $(window); |
| $w.scroll(function() { |
| var scrollTop = $w.scrollTop(); |
| var shouldBeFixed = $w.width() > 991 && scrollTop >= navHomeY - 10; |
| if (shouldBeFixed && !isFixed) { |
| nav.css({ |
| position: 'fixed', |
| top: 0, |
| left: nav.offset().left, |
| width: nav.width(), |
| }); |
| nav.addClass('fixed-navigation'); |
| isFixed = true; |
| } |
| else if (!shouldBeFixed && isFixed) |
| { |
| nav.css({ |
| position: 'static' |
| }); |
| nav.removeClass('fixed-navigation'); |
| isFixed = false; |
| } |
| }); |
| }); |
| </script> |
| |
| |
| <script type="text/javascript"> |
| var gaJsHost = (("https:" == document.location.protocol) ? "https://ssl." : "http://www."); |
| document.write(unescape("%3Cscript src='" + gaJsHost + "google-analytics.com/ga.js' type='text/javascript'%3E%3C/script%3E")); |
| |
| try { |
| var pageTracker = _gat._getTracker("UA-11583863-1"); |
| pageTracker._trackPageview(); |
| } catch(err) {} |
| </script> |
| |
| |
| </body> |
| </html> |