<html>
<head>
<base href="https://wiki.asterisk.org/wiki">
<link rel="stylesheet" href="/wiki/s/2036/1/7/_/styles/combined.css?spaceKey=TOP&forWysiwyg=true" type="text/css">
</head>
<body style="background: white;" bgcolor="white" class="email-body">
<div id="pageContent">
<div id="notificationFormat">
<div class="wiki-content">
<div class="email">
<h2><a href="https://wiki.asterisk.org/wiki/display/TOP/Code+Reviews">Code Reviews</a></h2>
<h4>Page <b>edited</b> by <a href="https://wiki.asterisk.org/wiki/display/~dlee">David M. Lee</a>
</h4>
<div id="versionComment">
<b>Comment:</b>
Added link to Git Hints and Usage<br />
</div>
<br/>
<h4>Changes (2)</h4>
<div id="page-diffs">
<table class="diff" cellpadding="0" cellspacing="0">
<tr><td class="diff-snipped" >...<br></td></tr>
<tr><td class="diff-unchanged" ># Developer deletes the topic branch, both locally and remotely <br> <br></td></tr>
<tr><td class="diff-deleted-lines" style="color:#999;background-color:#fdd;text-decoration:line-through;">The cheat sheet for the final bit of git magic is as follows: <br>{code:bash} <br># get latest changes from the server <br>git fetch origin <br># checkout the topic branch <br>git checkout topic-branch <br># rebase to master <br>git rebase origin/master <br># checkout master <br>git checkout master <br># merge changes from the topic branch. should be just a fast forward <br>git merge topic-branch <br># push the changes <br>git push <br># delete the local topic branch <br>git branch -d topic-branch <br># delete the remote topic branch <br>git push integration :topic-branch <br></td></tr>
<tr><td class="diff-added-lines" style="background-color: #dfd;">See [Git Hints and Usage] for some hints about using getting the most out of git. <br></td></tr>
<tr><td class="diff-unchanged" >{code} <br></td></tr>
</table>
</div> <h4>Full Content</h4>
<div class="notificationGreySide">
<p>We use <a href="https://code.asterisk.org/code/cru/browse/CR-ASTSCF" class="external-link" rel="nofollow">Crucible</a> for managing code reviews for Asterisk SCF. I recommend Asterisk SCF contributors quickly peruse the <a href="http://confluence.atlassian.com/display/CRUCIBLE/Crucible+User%27s+Guide" class="external-link" rel="nofollow">Crucible User's Guide</a> for details on working with the tool.</p>
<p>We use a slightly modified <a href="http://confluence.atlassian.com/display/CRUCIBLE/Agile+Permissions+Schemes+in+Crucible" class="external-link" rel="nofollow">Agile permission scheme</a> for our reviews. The modifications are:</p>
<ul>
<li>The Moderator role is disabled.</li>
<li>Only Asterisk SCF contributors may create or comment on reviews.
<ul>
<li>To become a contributor, please see the <a href="/wiki/display/TOP/Community" title="Community">Community</a> page for more information.</li>
</ul>
</li>
</ul>
<h1><a name="CodeReviews-Workflow"></a>Workflow</h1>
<p>By design, the workflow for Asterisk SCF code reviews is very open and non-restrictive. Any contributor can create a review from an uploaded patch, a commit changeset, a particular revision of a file in any of the release or integration repos.</p>
<p>However, most code reviews will follow a relatively simple process.</p>
<ol>
<li>Developer writes some code.
<ul>
<li>With sufficient <a href="/wiki/display/TOP/Asterisk+SCF+Testing" title="Asterisk SCF Testing">automated tests</a>, of course.</li>
</ul>
</li>
<li>Developer pushes code to an integration repo.</li>
<li><em>Repeat until code is ready for review.</em></li>
<li>Developer goes to <a href="http://code.asterisk.org" class="external-link" rel="nofollow">http://code.asterisk.org</a> and creates a code review for their changes.
<ul>
<li>When selecting reviewers, be courteous take a look at the <a href="https://code.asterisk.org/code/plugins/servlet/review-blockers/" class="external-link" rel="nofollow">review blockers report</a> to see if anyone's overloaded with reviews.</li>
</ul>
</li>
</ol>
<p>Then:</p>
<ol>
<li>Reviewers comment on the code for review.</li>
<li>Developer responds to comment.</li>
<li>Developer updates code in response to the review.
<ul>
<li>Or the developer can create a Jira issue for later action.</li>
</ul>
</li>
<li>When a reviewer runs out of things to say, the reviewer completes their review.
<ul>
<li>Crucible will automagically un-complete the review if anyone adds or updates the code in the review.</li>
</ul>
</li>
<li><em>Repeat until the code is awesome</em>.</li>
</ol>
<p>Finally:</p>
<ol>
<li>Developer summarizes the review.</li>
<li>Developer merges into the release master branch.
<ul>
<li>If it's reasonable to do this with a rebase, do so to avoid excessive merge commits. If the rebase goes wrong, then you can use <a href="http://www.kernel.org/pub/software/scm/git/docs/git-reflog.html" class="external-link" rel="nofollow">git reflog</a> and <a href="http://www.kernel.org/pub/software/scm/git/docs/git-reset.html" class="external-link" rel="nofollow">git reset --hard</a> to get back to your pre-rebase goodness.</li>
</ul>
</li>
<li>Developer deletes the topic branch, both locally and remotely</li>
</ol>
<p>See <a href="/wiki/display/TOP/Git+Hints+and+Usage" title="Git Hints and Usage">Git Hints and Usage</a> for some hints about using getting the most out of git.</p>
<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<script type="syntaxhighlighter" class="toolbar: false; theme: Confluence; brush: java; gutter: false"><![CDATA[]]></script>
</div></div>
</div>
<div id="commentsSection" class="wiki-content pageSection">
<div style="float: right;">
<a href="https://wiki.asterisk.org/wiki/users/viewnotifications.action" class="grey">Change Notification Preferences</a>
</div>
<a href="https://wiki.asterisk.org/wiki/display/TOP/Code+Reviews">View Online</a>
|
<a href="https://wiki.asterisk.org/wiki/pages/diffpagesbyversion.action?pageId=10650101&revisedVersion=3&originalVersion=2">View Changes</a>
|
<a href="https://wiki.asterisk.org/wiki/display/TOP/Code+Reviews?showComments=true&showCommentArea=true#addcomment">Add Comment</a>
</div>
</div>
</div>
</div>
</div>
</body>
</html>