<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/Code+Review" 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;">Code Review</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="CodeReview-Overview" class="diff-block-target diff-block-context"> <span class="diff-html-added" id="added-diff-0" style="font-size: 100%; background-color: #ddfade;">Overview</span> </h1>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">The Asterisk project is dedicated to ensuring that the changes made to it are of the best possible quality. Code reviews are one way the project helps to ensure that the changes made are well understood and do not have adverse effects. As an open source project, it is up to all of us to ensure that the changes made to Asterisk improve the project for the good of everyone.</span> </p>
<h1 id="CodeReview-CodeReviews" class="diff-block-target diff-block-context"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Code Reviews</span> </h1>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Code reviews are typically done through </span><a href="https://reviewboard.asterisk.org/" class="external-link" rel="nofollow" style="color: #326ca6; text-decoration: none"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Review Board</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">, although code review can also take place on issues in the </span><a href="https://issues.asterisk.org/" class="external-link" rel="nofollow" style="color: #326ca6; text-decoration: none"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Asterisk issue tracker</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">. Instructions on using Review Board can be found at </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;">Review Board Usage</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">. In general, we encourage all patch contributors to post their patches to Review Board, as this helps to ensure that all changes to Asterisk have been reviewed by multiple developers.</span> </p>
<table class="diff-macro diff-html-added diff-block-target diff-block-context" style="background-color: #f0f0f0;border: 1px solid #dddddd;margin: 10px 1px;padding: 0 2px 2px;width: 100%;background-color: #ddfade;border-color: #93c49f;">
<thead>
<tr>
<th class="diff-macro-title" style="background-color: transparent; text-align: left; font-weight: normal;padding: 5px;; font-size: 13px"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"><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</span></th>
</tr>
</thead>
<tbody>
<tr>
<td class="diff-macro-properties" style="background-color: #fafafa; padding: 0 0 0 5px; font-size: 12px; text-align: left;padding: 0; border: 1px solid #dddddd;; font-size: 13px">
<table>
<tbody>
<tr>
<td style="background-color: #fafafa; padding: 0 0 0 5px; font-size: 12px; text-align: left;; font-size: 13px"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">title</span></td>
<td style="background-color: #fafafa; padding: 0 0 0 5px; font-size: 12px; text-align: left;; font-size: 13px"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Licensing and Attribution</span></td>
</tr>
</tbody>
</table> </td>
</tr>
</tbody>
<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" style="font-size: 100%; background-color: #ddfade;">By posting a patch to Review Board, you agree that you are the author of the patch and that you have the license to contribute the code to the Asterisk project per the </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;">Digium License Agreement</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">. If you are not the author of the patch, please arrange for the author to either post the patch themselves or comment on the respective JIRA issue that you have permission to contribute the patch to the project.</span> </p> </td>
</tr>
</tbody>
</table>
<h2 id="CodeReview-ReviewWorkflow" class="diff-block-target diff-block-context"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Review Workflow</span> </h2>
<ol class="diff-block-target diff-block-context">
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">The patch author posts the patch to Review Board. The following should be filled in on the review:</span> <br />
<ul>
<li style="font-size: 13px"> <strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Summary: </span></strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">describe in brief what the patch does. If possible, it is useful to prefix the summary with the affected modules in Asterisk, e.g., chan_sip, app_queue, etc.</span> </li>
<li style="font-size: 13px"> <strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Bugs</span></strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">: provide the issue in JIRA that the patch resolves.</span> </li>
<li style="font-size: 13px"> <strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Groups: </span></strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">always specify the "asterisk-dev" group. This will cause the review to automatically generate an e-mail to the </span><a href="http://lists.digium.com/" class="external-link" rel="nofollow" style="color: #326ca6; text-decoration: none"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">asterisk-dev</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"> list.</span> </li>
<li style="font-size: 13px"> <strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">People:</span></strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"> often, there are subject matter experts who would be helpful in reviewing your issue. If you know of particular developers that you would like to review your patch, provide their username here. If a patch affects a module maintained by a </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;">community developer</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">, you should provide their name here.</span> </li>
</ul> </li>
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Other developers will review the patch. See the INSERT CHECK LIST here for common items that developers will look for.</span>
<ul>
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">If there are findings, the patch author should resolve them. Resolved findings should be closed out on the review.</span> </li>
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">When all findings are resolved, the patch author should re-submit the patch to the same review. See </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;">Review Board Usage</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"> for instructions on updating a review.</span> </li>
</ul> </li>
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">If a developer provides a "Ship It" for a review, that implies that the developer feels that the patch is ready for inclusion.</span>
<ul>
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">If a developer comments on a review that has a "Ship It", those findings should be resolved before the patch is included.</span> </li>
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Patches with a "Ship It" should, when possible, be given 24 hours before being committed. This allows developers in other time zones to comment on a shipped patch.</span> </li>
</ul> </li>
<li style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Once a patch has a "Ship It" and all findings are closed, the patch can be committed by a developer with commit access.</span> </li>
</ol>
<h2 id="CodeReview-WatchingCodeReviews" class="diff-block-target diff-block-context"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Watching Code Reviews</span> </h2>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">You can watch all code reviews that occur in the Asterisk project by subscribing to the </span><a href="http://lists.digium.com" class="external-link" rel="nofollow" style="color: #326ca6; text-decoration: none"><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">asterisk-dev</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"> mailing list. If you are the author of a patch or you have been listed in the </span><strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">People</span></strong><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;"> field, you will be automatically e-mailed whenever a change occurs in a code review.</span> </p>
<h1 id="CodeReview-AFinalNoteonCodeReviews" class="diff-block-target diff-block-context"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">A Final Note on Code Reviews</span> </h1>
<p class="diff-block-target diff-block-context" style="font-size: 13px"> <span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">Often, as developers, we take a personal pride in our work. This is a Good Thing: we should be proud of the work that we do. Submitting your work for peer review will subject your work to criticism. This is not a bad thing.</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;">The Asterisk project is bigger than any one developer. Ensuring quality for the project is the goal for everyone who participates in it, and sometimes that may result in findings against your code. It is not personal: the developers who participate in the Asterisk project are passionate about the success of the project and want the very best to go into it.</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;">When participating in a code review, please remember the </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;">Asterisk Community Code of Conduct</span></a><span class="diff-html-added" style="font-size: 100%; background-color: #ddfade;">, and remember that we're all trying to make Asterisk the best open source communications engine.</span> </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/Code+Review" 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=27820111" 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=27820111&revisedVersion=2&originalVersion=1" 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/Code+Review?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>