| --- |
| layout: docpage |
| |
| title: "Documentation" |
| |
| is_homepage: false |
| is_sphinx_doc: true |
| |
| doc-parent: "Contributing to Cassandra" |
| |
| doc-title: "Review Checklist" |
| doc-header-links: ' |
| <link rel="top" title="Apache Cassandra Documentation v4.0-alpha5" href="../index.html"/> |
| <link rel="up" title="Contributing to Cassandra" 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"/> |
| ' |
| doc-search-path: "../search.html" |
| |
| extra-footer: ' |
| <script type="text/javascript"> |
| var DOCUMENTATION_OPTIONS = { |
| URL_ROOT: "", |
| VERSION: "", |
| COLLAPSE_INDEX: false, |
| FILE_SUFFIX: ".html", |
| HAS_SOURCE: false, |
| SOURCELINK_SUFFIX: ".txt" |
| }; |
| </script> |
| ' |
| |
| --- |
| <div class="container-fluid"> |
| <div class="row"> |
| <div class="col-md-3"> |
| <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="../new/index.html">New Features in Apache Cassandra 4.0</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="../cql/index.html">The Cassandra Query Language (CQL)</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="../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">Contributing to Cassandra</a><ul class="current"> |
| <li class="toctree-l2"><a class="reference internal" href="gettingstarted.html">Getting Started</a></li> |
| <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> |
| <li class="toctree-l2"><a class="reference internal" href="documentation.html">Working on Documentation</a></li> |
| <li class="toctree-l2"><a class="reference internal" href="ci.html">Jenkins CI Environment</a></li> |
| <li class="toctree-l2"><a class="reference internal" href="dependencies.html">Dependency Management</a></li> |
| <li class="toctree-l2"><a class="reference internal" href="release_process.html">Release Process</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="../plugins/index.html">Third-Party Plugins</a></li> |
| <li class="toctree-l1"><a class="reference internal" href="../bugs.html">Reporting Bugs</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="content-container"> |
| |
| <div class="section" id="review-checklist"> |
| <span id="how-to-review"></span><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> |
| </div> |