| The [Melange project](MelangeIntro.md) has a variety of opportunities for the | |
| community to contribute, including: | |
| * [Filing an issue](http://code.google.com/p/soc/issues/entry) reporting a defect or requesting a new feature | |
| * Contributing a change to Melange's code implementing new behavior | |
| Each of these contributions is subject to review by other members of the | |
| Melange community. The guidelines for the review of various types of | |
| contributions are described here. | |
| ## CLA | |
| <h4>All contributors must <a href='ContributorLicenseAgreements.md'>sign the CLA</a> before we can accept their contributions.<h4> | |
| <h2>Committers</h2> | |
| Committers in the Melange community are those individuals who have been<br> | |
| made project members or project owners in the<br> | |
| <a href='http://code.google.com/p/soc/'>SoC project</a> on<br> | |
| <a href='http://code.google.com'>Google Code</a>. These individuals have been given<br> | |
| commit privileges as a result of their previous contributions to the<br> | |
| Melange community, usually in the form of<br> | |
| <a href='ContributionReviews#Patches.md'>patches</a>. These individuals are the<br> | |
| "gatekeepers" of the community, helping to insure that standards such<br> | |
| as <a href='PythonStyleGuide.md'>programming style</a> and<br> | |
| <a href='TestingGuidelines.md'>testing</a> that are important to the community as a<br> | |
| whole are adhered to by each contributor and for each contribution.<br> | |
| <br> | |
| <a href='Hidden comment: | |
| Each committer will: | |
| * be a project member (or possibly project owner) at http://code.google.com/p/soc/ | |
| * be a [MailingListsGuidelines#Developer_list developer mailing list] manager (or possibly list owner) at http://groups.google.com/group/melange-soc-dev | |
| * have an account at at [http://mail.google.com/a/googleopensourceprograms.com googleopensourceprograms.com] | |
| * be able to [ContributionReviews#Using_Rietveld_for_code_reviews conduct code reviews] at http://codereviews.googleopensourceprograms.com/ | |
| * have ACCESS template VOP (AUTOVOICE) in the [IrcChannelGuidelines `#melange` IRC channel] | |
| '></a><br> | |
| <br> | |
| <h3>Casual contributors</h3> | |
| Casual developers can contribute to the <a href='MelangeIntro.md'>Melange project</a> | |
| primarily through the <a href='ContributionReviews#Patches.md'>patch process</a> | |
| described below. Casual contributors still need to agree to the<br> | |
| <a href='ContributionTerms.md'>guidelines for contributors</a> before their patch<br> | |
| can be accepted and commmitted by a<br> | |
| <a href='ContributionReviews#Committer.md'>committer</a>.<br> | |
| <br> | |
| <h2>Code reviews</h2> | |
| Code reviews are handled in a variety of ways, depending on the size of<br> | |
| the change and the role of the contributor in the Melange community. Most<br> | |
| contributions will be in the form of small<br> | |
| <a href='ContributionReviews#Suggested_patches_from_casual_contributors.md'>patches</a> and<br> | |
| <a href='ContributionReviews#Reviews_after_committed_patches.md'>commits</a>. Larger<br> | |
| contributions, such as entire new features, probably require a different<br> | |
| approach, such as incremental patches and communication with the other<br> | |
| developers. These have similar review approaches, but tailored to the<br> | |
| specific situation.<br> | |
| <br> | |
| <h3>Code changes should be atomic</h3> | |
| The commit of a source code change should result in a source repository<br> | |
| where the software <a href='BuildingMelange.md'>builds</a> and passes any existing<br> | |
| <a href='TestingGuidelines.md'>tests</a>. So, all related changes need to be committed<br> | |
| in such a way that each patch passes the test suite. That usually means<br> | |
| that new functionality is introduced in one patch, 'enabled' in another one,<br> | |
| and the old functionality removed in a third one.<br> | |
| <br> | |
| <h3>Patches</h3> | |
| The term "patches" is used to refer to relatively small changes to the<br> | |
| code (and, yes, this definition is intentionally vague).<br> | |
| <br> | |
| Patches should be sent for review via our <a href='Gerrit.md'>Gerrit</a> instance.<br> | |
| <br> | |
| <h3>Patchfiles</h3> | |
| It is prefered that all Patches be sent via <a href='Gerrit.md'>Gerrit</a>, but if you need to<br> | |
| create a traditional Patchfile, you can follow these instructions:<br> | |
| <br> | |
| Usually these<br> | |
| are in the form of output from the <code>git diff</code> command.<br> | |
| <br> | |
| Patches should be sent for review to the<br> | |
| <a href='MailingListsGuidelines#Developer_list.md'>developer mailing list</a> and<br> | |
| presented in the "unified <code>diff</code>" format, which is de-facto standard<br> | |
| for many tools and the output from the <code>git diff</code> command (the same<br> | |
| format as that produced by <code>diff -uNr</code>). Using Git:<br> | |
| <br> | |
| <pre><code>git diff --no-prefix > your-patch-name.patch<br> | |
| </code></pre> | |
| The reviewer can then apply the patch using something like:<br> | |
| <br> | |
| <pre><code>patch -p0 < your-patch-name.patch<br> | |
| </code></pre> | |
| <h4>Patch emails</h4> | |
| Patch emails sent to the developer mailing list should begin with<br> | |
| <code>[PATCH]</code> as the first text in the email subject line, followed by a<br> | |
| brief (one line) description of the change.<br> | |
| <br> | |
| For "small" patches (for some definition of "small", please use good<br> | |
| judgment), include the diff in the email body, to facilitate inline<br> | |
| commenting in the code review email thread. For <i>all</i> patches, including<br> | |
| both "small" and "non small" ones, include the diff itself as an email<br> | |
| attachment, to make it easier for reviewers to patch the change into a<br> | |
| working copy with <code>patch</code>. Such a diff attachment can be produced with<br> | |
| the methods described above.<br> | |
| <br> | |
| <a href='Hidden comment: | |
| ==== Suggested patches from casual contributors ==== | |
| The Melange community is interested in patches and other contributions | |
| from casual developers. To that end, casual developers can submit | |
| patches for review in a couple of ways: | |
| * attach a patch to an [http://code.google.com/p/soc/issues/list issue in the issue tracker] | |
| * mailing a patch to the [MailingListsGuidelines#Developer_list developer mailing list] (membership in the list is required to post to it; see the [MailingListsGuidelines#Developer_list list guidelines]) | |
| Committers can then discuss the contribution with the author on the | |
| [MailingListsGuidelines#Developer_list developer mailing list], or on the | |
| [MailingListsGuidelines#General_discussion_list general discussion list] | |
| if the contributor is not on the developer list. Since the | |
| [MelangeIntro Melange project] and the [SoCOverview SoC framework] is mostly | |
| about code, there are likely to be at least a few code discussions on the | |
| general announcement list. Serious discussions will likely result in the | |
| casual contributor being invited to the developer list to move the discussion | |
| there. | |
| Once the patch is accepted, a committer will patch a working copy and commit | |
| the changed (or added or deleted) source files to the repository, | |
| supplying a suitable commit log message. | |
| '></a><br> | |
| <br> | |
| <h4>Code Review</h4> | |
| Code review happens in the <a href='Gerrit.md'>Gerrit</a> system.<br> | |
| <br> | |
| All patches need a +2 from a Comitter before they can be submitted.<br> | |
| <br> | |
| <a href='Hidden comment: | |
| ==== Reviews after committed patches ==== | |
| Contributors with [ContributionReviews#Committers commit access] can submit | |
| changes prior to review. The participants on the | |
| [MailingListsGuidelines#Developer_list developer mailing list] are then | |
| encouraged to review the change, which will be present in the commit | |
| email that was sent to the mailing list. At least one other person with | |
| commit access should acknowledge the change with a brief "looks good to me" (LGTM) | |
| message in response (sent back to the list). This does of course not prevent | |
| other list participants from being able to make suggestions, request | |
| changes, etc. | |
| If one or more of the list participants requests changes to the submitted | |
| code, they can either discuss it and have the original committer submit a | |
| subsequent change, or the they can make the change themselves. Please | |
| avoid conflict by trying to reach a consensus before making commits. One | |
| way to do this is to | |
| [ContributionReviews#Patches follow the patch contribution method] for | |
| reviewing a change on the developer mailing list, even if the contributor | |
| has commit access. | |
| ===== Review after commit using Google Code code reviews ===== | |
| Google Code now has a feature that enables | |
| [http://code.google.com/p/support/wiki/CodeReviews code reviews of committed code]. | |
| This feature can also be used by project owners and members to do review-after-commit | |
| style code reviews. | |
| '></a><br> | |
| <br> | |
| <h3>Committing</h3> | |
| Once all of the<br> | |
| [TestingGuidelines#All_changes_should_pass_smoke_tests_before_review<br> | |
| pre-commit tests] pass <b>and</b> the [ContributionReviews#Patches patch<br> | |
| has been code-reviewed] a <a href='ContributionReviews#Committers.md'>committer</a> | |
| (who may or<br> | |
| [ContributionReviews#Suggested_patches_from_casual_contributors may<br> | |
| not] be the actual contributor), the submit button in <a href='Gerrit.md'>Gerrit</a> or `git<br> | |
| merge && git push` can be used to commit the change into the<br> | |
| <a href='http://code.google.com/p/soc/source/browse'>SoC project repository</a>.<br> | |
| <br> | |
| <h4>commit messages</h4> | |
| Committers should use the template below for <code>git commit</code> log messages:<br> | |
| <br> | |
| <pre><code>One line, under 68 characters giving a short description of the change<br> | |
| <br> | |
| After a blank line, a more detailed log message explaining the reason<br> | |
| of the change (not necessarily what was changed, as that can be seen<br> | |
| by readin the patch). Please wrap your lines at 70 characters again.<br> | |
| </code></pre> | |
| If committing contributor patch (outside of Gerrit) please include:<br> | |
| <br> | |
| <pre><code>Reviewed by: Rick Reviewer<br> | |
| </code></pre> | |
| Note that previously the person listed with "Patch by:" was the<br> | |
| contributor (that is, author) of the patch, however we have switched<br> | |
| to Git and now we are committing patch from contributors as if<br> | |
| they would have commit access using command:<br> | |
| <br> | |
| <pre><code>git commit -u "FirstName LastName <somebody@example.com>"<br> | |
| </code></pre> | |
| Patches from<br> | |
| [ContributionReviews#Suggested_patches_from_casual_contributors casual<br> | |
| contributors] will, by definition, need to be committed by someone<br> | |
| other than the author but Git repository history will indicate<br> | |
| contributor as committer.<br> | |
| <br> | |
| <h2>Documentation</h2> | |
| Code files should include enough internal documentation in the form of<br> | |
| <code>__doc__</code> strings, comments and functions with self-explanatory names<br> | |
| (see PythonStyleGuide) that the automatic documentation suffices.<br> | |
| This is the recommended way to ensure that code documentation and the<br> | |
| code itself stay in step.<br> | |
| <br> | |
| The wiki is in the transition zone between dialog (IRC and mailing<br> | |
| lists) and formally reviewed content (code and its comments). Changes<br> | |
| to the wiki are 'review after commit'. Any committer can log comments<br> | |
| for a wiki page and participate in review. At the moment rights to<br> | |
| create pages and edit pages directly are restricted to committers and<br> | |
| regular contributors to documentation.<br> | |
| <br> | |
| <hr /> | |
| <i>Copyright 2008 Google Inc.</i> | |
| <i>This work is licensed under a</i> | |
| <a href='http://soc.googlecode.com/svn/wiki/html/licenses/cc-by-attribution-2_5.html'>Creative Commons Attribution 2.5 License</a>.<br> | |
| <a href='http://creativecommons.org/licenses/by/2.5/'><img src='http://soc.googlecode.com/svn/wiki/html/licenses/cc-by-2_5-88x31.png' /></a> |