<html>
<head>
<meta name="viewport" content="width=device-width" />
<base href="https://wiki.asterisk.org/wiki" />
<style type="text/css">
body, #email-content, #email-content-inner { font-family: Arial,FreeSans,Helvetica,sans-serif; }
body, p, blockquote, pre, code, td, th, li, dt, dd { font-size: 13px; }
small { font-size: 11px; }
body { width:100% !important; -webkit-font-smoothing: antialiased; }
body,
#email-wrapper { background-color: #f0f0f0; }
#email-wrapper-inner { padding: 20px; text-align: center; }
#email-content-inner { background-color: #fff; border: 1px solid #bbb; color: $menuTxtColour; padding:20px; text-align:left; }
#email-wrapper-inner > table { width: 100%; }
#email-wrapper-inner.thin > table { margin: 0 auto; width: 50%; }
#email-footer { padding: 0 16px 32px 16px; margin: 0; }
.email-indent { margin: 8px 0 16px 0; }
.email-comment { margin: 0 0 0 56px; }
.email-comment.removed { background-color: #ffe7e7; border: 1px solid #df9898; padding: 0 8px;}
#email-title-avatar { text-align: left; vertical-align: top; width: 48px; padding-right: 8px; }
#email-title-flavor { margin: 0; padding: 0 0 4px 0; }
#email-title-heading { font-size: 16px; line-height: 20px; min-height: 20px; margin: 0; padding: 0; }
#email-title .icon { border: 0; padding: 0 5px 0 0; text-align: left; vertical-align: middle; }
#email-actions { border-top: 1px solid #bbb; color: #505050; margin: 8px 0 0 0; padding: 0; }
#email-actions td { padding-top: 8px; }
#email-actions .left { max-width: 45%; text-align: left; }
#email-actions .right { text-align: right; }
.email-reply-divider { border-top: 1px solid #bbb; color: #505050; margin: 32px 0 8px 0; padding: 8px 0; }
.email-section-title { border-bottom: 1px solid #bbb; margin: 8px 0; padding: 8px 0 0 0; }
.email-metadata { color: #505050; }
a { color: #326ca6; text-decoration: none; }
a:hover { color: #336ca6; text-decoration: underline; }
a:active {color: #326ca6; }
a.email-footer-link { color: #505050; font-size: 11px; }
.email-item-list { list-style: none; margin: 4px 0; padding-left: 0; }
.email-item-list li { list-style: none; margin: 0; padding: 4px 0; }
.email-list-divider { color: #505050; padding: 0 0.35em; }
.email-operation-icon { padding-right: 5px; }
.avatar { -ms-interpolation-mode: bicubic; border-radius: 3px;}
.avatar-link { margin: 2px; }
.tableview th { border-bottom: 1px solid #69C; font-weight: bold; text-align: left; }
.tableview td { border-bottom: 1px solid #bbbbbb; text-align: left; padding: 4px 16px 4px 0; }
.aui-message { margin: 1em 0; padding: 8px; }
.aui-message.info { background-color: #e0f0ff; border: 1px solid #9eb6d4; }
.aui-message.success { background-color: #ddfade; border: 1px solid #93c49f; }
.aui-message.error,
.aui-message.removed { background-color: #ffe7e7; border: 1px solid #df9898; color: #000; }
.call-to-action-table { margin: 10px 1px 1px 1px;}
.call-to-cancel-container, .call-to-action-container { padding: 5px 20px; }
.call-to-cancel-container { border: 1px solid #aaa; background-color: #eee; border-radius: 3px; }
.call-to-cancel-container a.call-to-cancel-button { background-color: #eee; font-size: 14px; line-height: 1; padding: 0; margin: 0; color: #666; font-family: sans-serif;}
.call-to-action-container { border: 1px solid #486582; background-color: #3068A2; border-radius: 3px; padding: 4px 10px; }
.call-to-action-container a.call-to-action-button { background-color: #3068A2; font-size: 14px; line-height: 1; padding: 0; margin: 0; color: #fff; font-weight: bold; font-family: sans-serif; }
/** The span around the inline task checkbox image */
.diff-inline-task-overlay {
display: inline-block;
text-align: center;
height: 1.5em;
padding: 5px 0px 1px 5px;
margin-right: 5px;
/** Unfortunately, the negative margin-left is stripped out in gmail */
margin-left: -5px;
}
@media handheld, only screen and (max-device-width: 480px) {
div, a, p, td, th, li, dt, dd { -webkit-text-size-adjust: auto; }
small, small a { -webkit-text-size-adjust: 90%; }
td[id=email-wrapper-inner] { padding: 2px !important; }
td[id=email-content-inner] { padding: 8px !important; }
td[id="email-wrapper-inner"][class="thin"] > table { text-align: left !important; width: 100% !important; }
td[id=email-footer] { padding: 8px 12px !important; }
div[class=email-indent] { margin: 8px 0px !important; }
div[class=email-comment] { margin: 0 !important; }
p[id=email-title-flavor] a { display: block; } /* puts the username and the action on separate lines */
p[id=email-permalink] { padding: 4px 0 0 0 !important; }
table[id=email-actions] td { padding-top: 0 !important; }
table[id=email-actions] td.right { text-align: right !important; }
table[id=email-actions] .email-list-item { display: block; margin: 1em 0 !important; word-wrap: normal !important; }
span[class=email-list-divider] { display: none; }
}
</style>
</head>
<body style="font-family: Arial, FreeSans, Helvetica, sans-serif; font-size: 13px; width: 100%; -webkit-font-smoothing: antialiased; background-color: #f0f0f0">
<table id="email-wrapper" width="100%" cellspacing="0" cellpadding="0" border="0" style="background-color: #f0f0f0">
<tbody>
<tr valign="middle">
<td id="email-wrapper-inner" style="font-size: 13px; padding: 20px; text-align: center">
<table id="email-content" cellspacing="0" cellpadding="0" border="0" style="font-family: Arial, FreeSans, Helvetica, sans-serif; width: 100%">
<tbody>
<tr valign="top">
<td id="email-content-inner" align="left" style="font-family: Arial, FreeSans, Helvetica, sans-serif; font-size: 13px; background-color: #fff; border: 1px solid #bbb; padding: 20px; text-align: left">
<table id="email-title" cellpadding="0" cellspacing="0" border="0" width="100%">
<tbody>
<tr>
<td id="email-title-avatar" rowspan="2" style="font-size: 13px; text-align: left; vertical-align: top; width: 48px; padding-right: 8px"> <img class="avatar" src="cid:avatar_ce51dcf276530e4a4b00548e2a6d0905" border="0" height="48" width="48" style="-ms-interpolation-mode: bicubic; border-radius: 3px" /> </td>
<td valign="top" style="font-size: 13px">
<div id="email-title-flavor" class="email-metadata" style="margin: 0; padding: 0 0 4px 0; color: #505050">
<a href=" https://wiki.asterisk.org/wiki/display/~mjordan " style="color:#326ca6;text-decoration:none;; color: #326ca6; text-decoration: none">Matt Jordan</a> edited the page:
</div> </td>
</tr>
<tr>
<td valign="top" style="font-size: 13px"> <h2 id="email-title-heading" style="font-size: 16px; line-height: 20px; min-height: 20px; margin: 0; padding: 0"> <a href="https://wiki.asterisk.org/wiki/display/AST/Review+Board+Usage" style="color: #326ca6; text-decoration: none"> <img class="icon" src="cid:page-icon" alt="" style="border: 0; padding: 0 5px 0 0; text-align: left; vertical-align: middle" /> <strong style="font-size:16px;line-height:20px;vertical-align:top;">Review Board Usage</strong> </a> </h2> </td>
</tr>
</tbody>
</table>
<div class="email-indent" style="margin: 8px 0 16px 0">
<div class="email-diff">
<div id="page-diffs" class="wiki-content">
<h1 id="ReviewBoardUsage-UsageGuidelines" class="diff-block-target diff-block-context"> <span class="diff-html-changed" id="changed-diff-0" style="background-color: #d6f0ff;">Usage Guidelines</span> </h1>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <a href="https://issues.asterisk.org/jira/" class="external-link" rel="nofollow" style="color: #326ca6; text-decoration: none">JIRA</a> and <a href="https://reviewboard.asterisk.org" class="external-link" rel="nofollow" style="color: #326ca6; text-decoration: none"><span class="diff-html-removed" id="removed-diff-0" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">Reviewboard</span></a><span class="diff-html-removed" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;"> </span><a href="https://reviewboard.asterisk.org" class="external-link" rel="nofollow" style="color: #326ca6; text-decoration: none"><span class="diff-html-added" id="added-diff-0" style="font-size: 100%; background-color: #ddfade;">Review Board</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"> </span>are both utilities that the Asterisk development community uses to help track and review code being written for Asterisk. <span class="diff-html-removed" id="removed-diff-1" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">Since both systems are used for posting patches, it is worth discussing when it is appropriate to use reviewboard and when not.</span> </p>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <span class="diff-html-removed" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">Here are the situations in which it is appropriate to post code to reviewboard:</span> </p>
<ul class="diff-block-target diff-block-context">
<li style="font-size: 13px"> <span class="diff-html-removed" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">A committer or developer with access to reviewboard has a patch that they would like to get some feedback on before merging into one of the main branches.</span> </li>
<li style="font-size: 13px"> <span class="diff-html-removed" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">A committer or bug marshal has requested a contributor to post their patch from JIRA on reviewboard to aid in the review process. This typically happens with complex code contributions where reviewboard can help aid in providing feedback.</span> </li>
</ul>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <span class="diff-html-removed" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">We do encourage all interested parties to participate in the review process. However, aside from the cases mentioned above, we prefer that all code submissions first go through JIRA.</span><span class="diff-html-added" id="added-diff-1" style="font-size: 100%; background-color: #ddfade;">Patches should generally be first attached to an issue in JIRA, as this will ensure that the issue is tracked appropriately and that proper attribution occurs. Review Board is a code review tool to help developers review a patch for submission.</span> </p>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">This page provides guidelines for using Review Board.</span> </p>
<h1 id="ReviewBoardUsage-PostingCodetoReviewBoard" class="diff-block-target diff-block-context"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Posting Code to Review Board</span> </h1>
<table class="diff-macro diff-block-target diff-block-context" style="background-color: #f0f0f0;border: 1px solid #dddddd;margin: 10px 1px;padding: 0 2px 2px;width: 100%;">
<thead>
<tr>
<th class="diff-macro-title" style="background-color: transparent; text-align: left; font-weight: normal;padding: 5px;; font-size: 13px"><span class="icon macro-placeholder-icon" style="background-color: ;line-height: 20px;"><img src="https://wiki.asterisk.org/wiki/s/en_GB-1988229788/4252/6ac85e9b14675c5514a674e1aecae99c9505ed36.48/_/images/icons/macrobrowser/dropdown/note.png" style="padding-right: 5px; vertical-align: text-bottom;" /> </span>Note</th>
</tr>
</thead>
<tbody>
<tr>
<td class="diff-macro-body" style="background-color: #fff;border: 1px solid #dddddd;padding: 10px;; font-size: 13px"> <p style="font-size: 13px">It is acceptable for a committer to post patches to <span class="diff-html-removed" id="removed-diff-2" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">reviewboard </span><span class="diff-html-added" id="added-diff-2" style="font-size: 100%; background-color: #ddfade;">Review Board </span>before they are complete to get some feedback on the approach being taken. However, if the code is not yet ready to be merged, it must be documented as such.</p> <p style="font-size: 13px">A review request with a patch proposed for merging should have documented testing and should not have <span class="diff-html-removed" id="removed-diff-3" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">blatant coding guidelines violations. Lack of these things is careless and shows disrespect for those reviewing your code.</span> </p> </td>
</tr>
</tbody>
</table>
<h1 id="ReviewboardUsage-PostingCodetoReviewboard" class="diff-block-target diff-block-context"> <span class="diff-html-removed" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">Posting Code to Reviewboard</span> </h1>
<table class="diff-macro diff-block-target diff-block-context" style="background-color: #f0f0f0;border: 1px solid #dddddd;margin: 10px 1px;padding: 0 2px 2px;width: 100%;">
<tbody>
<tr>
<td class="diff-macro-body" style="background-color: #fff;border: 1px solid #dddddd;padding: 10px;; font-size: 13px"> <p style="font-size: 13px"> <span class="diff-html-added" id="added-diff-4" style="font-size: 100%; background-color: #ddfade;">blatant </span><a class="confluence-link unresolved" href="#" style="color: #326ca6; text-decoration: none"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Coding Guidelines</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"> violations. If a patch has substantial issues, the review will be closed and you will be asked to re-submit it once it conforms to the project guidelines.</span> </p> </td>
</tr>
</tbody>
</table>
<h3 id="ReviewBoardUsage-Usingpost-review" class="diff-block-target diff-block-context"> <span class="diff-html-changed" id="changed-diff-2" style="background-color: #d6f0ff;">Using post-review</span> </h3>
<p class="diff-block-target diff-block-context" style="font-size: 13px">The easiest way to post a patch to <span class="diff-html-removed" id="removed-diff-4" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">reviewboard </span><span class="diff-html-added" id="added-diff-5" style="font-size: 100%; background-color: #ddfade;">Review Board </span>is by using the post-review tool. Install it using <code style="font-size: 13px">easy_install</code>.</p>
<p class="diff-context-placeholder" style="font-size: 13px">...</p>
<p class="diff-block-target" style="font-size: 13px">Essentially, post-review is a script that will take the output <span class="diff-html-removed" id="removed-diff-5" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">of "</span><span class="diff-html-added" id="added-diff-6" style="font-size: 100%; background-color: #ddfade;">of </span><code style="font-size: 13px"><span class="diff-html-changed" id="changed-diff-3" style="background-color: #d6f0ff;">svn diff</span></code><span class="diff-html-removed" id="removed-diff-6" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">" </span>and create a review request out of it for you. <span class="diff-html-removed" id="removed-diff-7" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">So, once </span><span class="diff-html-added" id="added-diff-7" style="font-size: 100%; background-color: #ddfade;">Once </span>you have a working copy with the changes you expect in the output of <span class="diff-html-removed" id="removed-diff-8" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">"</span><code style="font-size: 13px"><span class="diff-html-changed" id="changed-diff-4" style="background-color: #d6f0ff;">svn diff</span></code><span class="diff-html-removed" id="removed-diff-9" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">"</span>, <span class="diff-html-removed" id="removed-diff-10" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">you just </span>run the following command:</p>
<p class="diff-context-placeholder" style="font-size: 13px">...</p>
<p class="diff-block-target" style="font-size: 13px">If it complains about not knowing which <span class="diff-html-removed" id="removed-diff-11" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">reviewboard </span><span class="diff-html-added" id="added-diff-8" style="font-size: 100%; background-color: #ddfade;">Review Board </span>server to use, add the server option:</p>
<table class="diff-macro diff-block-context" style="background-color: #f0f0f0;border: 1px solid #dddddd;margin: 10px 1px;padding: 0 2px 2px;width: 100%;">
<thead>
<tr>
<th class="diff-macro-title" style="background-color: transparent; text-align: left; font-weight: normal;padding: 5px;; font-size: 13px"><span class="icon macro-placeholder-icon" style="background-color: ;line-height: 20px;"><img src="https://wiki.asterisk.org/wiki/s/en_GB-1988229788/4252/6ac85e9b14675c5514a674e1aecae99c9505ed36.48/_/plugins/servlet/confluence/placeholder/macro-icon?name=code" style="padding-right: 5px; vertical-align: text-bottom;" /> </span>Code Block</th>
</tr>
</thead>
<tbody>
<tr>
<td class="diff-macro-body" style="background-color: #fff;border: 1px solid #dddddd;padding: 10px;; font-size: 13px"> <pre style="font-size: 13px"> $ post-review --server=https://reviewboard.asterisk.org
</pre> </td>
</tr>
</tbody>
</table>
<h3 id="ReviewBoardUsage-DealingwithNewFiles" class="diff-block-target"> <span class="diff-html-changed" id="changed-diff-5" style="background-color: #d6f0ff;">Dealing with New Files</span> </h3>
<p class="diff-block-context" style="font-size: 13px">I have one final note about an oddity with using post-review. If you maintain your code in a team branch, and the new code includes new files, there are some additional steps you must take to get post-review to behave properly.</p>
<p class="diff-context-placeholder" style="font-size: 13px">...</p>
<p class="diff-block-target" style="font-size: 13px">Now, the code is merged into your working copy. However, for a new file, subversion treats it as a copy of existing content and not new content, so new files don't show up <span class="diff-html-removed" id="removed-diff-12" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">in "</span><span class="diff-html-added" id="added-diff-9" style="font-size: 100%; background-color: #ddfade;">in </span><code style="font-size: 13px"><span class="diff-html-changed" id="changed-diff-6" style="background-color: #d6f0ff;">svn diff</span></code><span class="diff-html-removed" id="removed-diff-13" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">" </span>at this point. To get it to show up in the diff, use the following commands so svn treats it as new content and publishes it in the diff:</p>
<p class="diff-context-placeholder" style="font-size: 13px">...</p>
<p class="diff-block-context" style="font-size: 13px">Now, it should work, and you can run "post-review" as usual.</p>
<h3 id="ReviewBoardUsage-UpdatingPatchonExistingReviewRequest" class="diff-block-target"> <span class="diff-html-changed" id="changed-diff-7" style="background-color: #d6f0ff;">Updating Patch on Existing Review Request</span> </h3>
<p class="diff-block-target diff-block-context" style="font-size: 13px">Most of the time, a patch on <span class="diff-html-removed" id="removed-diff-14" style="font-size: 100%; background-color: #ffe7e7; text-decoration: line-through;">reviewboard </span><span class="diff-html-added" id="added-diff-10" style="font-size: 100%; background-color: #ddfade;">Review Board </span>will require multiple iterations before other sign off on it being ready to be merged. To update the diff for an existing review request, you can use post-review and the -r option. Apply the current version of the diff to a working copy as described above, and then run the following command:</p>
<p class="diff-context-placeholder" style="font-size: 13px">...</p>
</div>
</div>
</div>
<table id="email-actions" class="email-metadata" cellspacing="0" cellpadding="0" border="0" width="100%" style="border-top: 1px solid #bbb; color: #505050; margin: 8px 0 0 0; padding: 0; color: #505050">
<tbody>
<tr>
<td class="left" valign="top" style="font-size: 13px; padding-top: 8px; max-width: 45%; text-align: left"> <span class="email-list-item"><a href="https://wiki.asterisk.org/wiki/display/AST/Review+Board+Usage" style="color: #326ca6; text-decoration: none">View Online</a> </span> <span class="email-list-divider" style="color: #505050; padding: 0 0.350em">·</span> <span class="email-list-item"><a href="https://wiki.asterisk.org/wiki/plugins/likes/like.action?contentId=4816917" style="color: #326ca6; text-decoration: none">Like</a> </span> <span class="email-list-divider" style="color: #505050; padding: 0 0.350em">·</span> <span class="email-list-item"><a href="https://wiki.asterisk.org/wiki/pages/diffpagesbyversion.action?pageId=4816917&revisedVersion=9&originalVersion=8" style="color: #326ca6; text-decoration: none">View Changes</a> </span> <span class="email-list-divider" style="color: #505050; padding: 0 0.350em">·</span> <span class="email-list-item"><a href="https://wiki.asterisk.org/wiki/display/AST/Review+Board+Usage?showComments=true&showCommentArea=true#addcomment" style="color: #326ca6; text-decoration: none">Add Comment</a> </span> </td>
<td class="right" width="50%" valign="top" style="font-size: 13px; padding-top: 8px; text-align: right"> <span class="email-list-item"><a href="https://wiki.asterisk.org/wiki/users/removespacenotification.action?spaceKey=AST" style="color: #326ca6; text-decoration: none">Stop watching space</a> </span> <span class="email-list-divider" style="color: #505050; padding: 0 0.350em">·</span> <span class="email-list-item"><a href="https://wiki.asterisk.org/wiki/users/editmyemailsettings.action" style="color: #326ca6; text-decoration: none">Manage Notifications</a> </span> </td>
</tr>
</tbody>
</table> </td>
</tr>
</tbody>
</table> </td>
</tr>
<tr>
<td id="email-footer" align="center" style="font-size: 13px; padding: 0 16px 32px 16px; margin: 0"> <small style="font-size: 11px"> This message was sent by <a class="email-footer-link" style="color:#505050;font-size:11px;text-decoration:none;; color: #326ca6; text-decoration: none; color: #505050; font-size: 11px" href="http://www.atlassian.com/software/confluence">Atlassian Confluence</a> 5.1.5, <a class="email-footer-link" style="color:#505050;font-size:11px;text-decoration:none;; color: #326ca6; text-decoration: none; color: #505050; font-size: 11px" href="http://www.atlassian.com/software/confluence/overview/team-collaboration-software?utm_source=email-footer">Team Collaboration Software</a> </small> </td>
</tr>
</tbody>
</table>
</body>
</html>