ContributionReviews.wiki - wiki - Git at Google

 #summary Guidelines for contributing to Melange
 #labels Contents-Skeleton,Phase-Guidelines

 The [MelangeIntro Melange project] has a variety of opportunities for the
 community to contribute, including:

  * documenting [http://code.google.com/p/soc/w/list?can=2&q=phase:Requirements feature requirements]
  * designing the [http://code.google.com/p/soc/w/list?can=2&q=phase:Design%20design:Model database schema] and different [http://code.google.com/p/soc/w/list?can=2&q=phase:Design%20design:View Views]
  * writing [http://code.google.com/p/soc/w/list?can=2&q=phase:Implementation source code]

 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.

 == Committers ==

 Committers in the Melange community are those individuals who have been
 made project members or project owners in the
 [http://code.google.com/p/soc/ SoC project] on
 [http://code.google.com Google Code].  These individuals have been given
 commit privileges as a result of their previous contributions to the
 Melange community, usually in the form of
 [ContributionReviews#Patches patches].  These individuals are the
 "gatekeepers" of the community, helping to insure that standards such
 as [PythonStyleGuide programming style] and
 [TestingGuidelines testing] that are important to the community as a
 whole are adhered to by each contributor and for each contribution.

 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]

 === Casual contributors ===

 Casual developers can contribute to the [MelangeIntro Melange project]
 primarily through the [ContributionReviews#Patches patch process]
 described below.  Casual contributors still need to agree to the
 [ContributionTerms guidelines for contributors] before their patch
 can be accepted and commmitted by a
 [ContributionReviews#Committer committer].

 == Code reviews ==

 Code reviews are handled in a variety of ways, depending on the size of
 the change and the role of the contributor in the Melange community.  Most
 contributions will be in the form of small
 [ContributionReviews#Suggested_patches_from_casual_contributors patches] and
 [ContributionReviews#Reviews_after_committed_patches commits]. Larger
 contributions, such as entire new features, probably require a different
 approach, such as development in a
 [ContributionReviews#Branches development branch].  These have similar
 review approaches, but tailored to the specific situation.

 === Code changes should be atomic ===

 The commit of a source code change should result in a source repository
 where the software [BuildingMelange builds] and passes any existing
 [TestingGuidelines tests].  So, all related changes need to be committed
 in a single patch, a single commit, or, in the case of branches, a
 single merge.

 === Patches ===

 The term "patches" is used to refer to relatively small changes to the
 code (and, yes, this definition is intentionally vague).  These can either
 be in the form of actual output from the `diff` command, or they can be
 the contents of the commit log message sent to the
 [MailingListsGuidelines#Developer_list developer mailing list] when a
 [ContributionReviews#Committers committer] commits a change using `svn`.

 Patches should be presented for review in the "unified `diff`" format,
 which is the output resulting from the `svn diff` command (the same format
 as that produced by`diff -uNr`, but generated automatically from the
 altered file in the working copy and the original file in the `.svn`
 subdirectory).  This is also the same format as the diff contained in the
 commit log messages emailed to the developer mailing list.

 One special note about creating patches:  please create the patch from
 the `/trunk/` directory (or the `/wiki/` directory if it is a documentation
 change), so that the reviewers and potential committers can apply the
 patch from the base of their `/trunk/` working copy.  For example, for
 a checkout of `https://soc.googlecode.com/svn/trunk/` to `~/svn/soc`,
 create the code review patch with something like:

 {{{
 cd ~/svn/soc
 svn diff soc/models/new_model.py soc/views/deleted_model.py > my_change.diff
 }}}

 In this example, `new_model.py` would be a newly-added module, and
 `deleted_model.py` would be a just-deleted module.  The reviewer can
 then apply the patch using something like:

 {{{
 cd ~/svn/soc
 patch -p0 < my_change.diff
 }}}

 The same `/trunk/` directory root approach should be used with
 [http://codereviews.googleopensourceprograms.com/static/upload.py `upload.py`]
 to submit
 [ContributionReviews#Using_Rietveld_for_code_reviews Rietveld code reviews].

 ==== Patch emails ====

 Patch emails sent to the developer mailing list should begin with
 `[PATCH]` as the first text in the email subject line, followed by a
 brief (one line) description of the change.

 For "small" patches (for some definition of "small", please use good
 judgment), include the diff in the email body, to facilitate inline
 commenting in the code review email thread.  For _all_ patches, including
 both "small" and "non small" ones, include the diff itself as an email
 attachment, to make it easier for reviewers to patch the change into a
 working copy with `patch`.  Such a diff attachment can be produced with
 something like:

 {{{
 svn diff some_module.py > some_module.diff
 }}}

 For multiple files changed in the same patch, use something like:

 {{{
 svn diff some_module.py some_other_module.py > my_new_feature.diff
 }}}

 Alternately, committers can use
 [http://codereviews.googleopensourceprograms.com/static/upload.py `upload.py`]
 to submit
 [ContributionReviews#Using_Rietveld_for_code_reviews Rietveld code reviews].

 ==== Using Rietveld for code reviews ====

 An instance of
 [http://code.google.com/p/rietveld/ Guido van Rossum's Rietveld]
 code review tool has been set up at
 http://codereviews.googleopensourceprograms.com for use by
 [ContributionReviews#Committers Melange committers].  An account on
 [http://mail.google.com/a/googleopensourceprograms.com googleopensourceprograms.com]
 is required in order to post code review requests and to provide code
 review comments.

 Patches supplied via the Rietveld web form should be created following
 [ContributionReviews#Patches these instructions].  To use the
 [http://codereviews.googleopensourceprograms.com/static/upload.py `upload.py`]
 script provided by Rietveld to start a code review, do something like this
 and respond to the prompts:

 {{{
 cd ~/svn/soc
 upload.py --server=codereviews.googleopensourceprograms.com \
   -- soc/models/new_model.py soc/models_deleted_model.py
 }}}

 Note that a copy of the `upload.py` script can be modified to make the
 default server be `codereviews.googleopensourceprograms.com`, to avoid
 constantly having to supply the `--server` option.

 To upload an updated diff for an existing code review in progress, simply
 supply the extra `--issue=<the existing issue number>` option.  Rietveld
 will then display the deltas between the previously supplied diff and the
 new one.

 ==== 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] _(not yet enabled at this time)_
  * 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 `svn` repository,
 supplying a suitable commit log message.

 ==== 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 `svn` 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"
 message in response (sent back to the list).  This does _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.

 === Other code review possibilities ===

 In the future, tools like
 [http://code.google.com/p/gvn/ Gvn Versions Nicely] might be used for
 "review then commit" code reviews that make use of a "change branch" model.

 === Committing ===

 Once all of the
 [TestingGuidelines#All_changes_should_pass_smoke_tests_before_review pre-commit tests]
 pass and the [ContributionReviews#Patches patch has been code-reviewed]
 a [ContributionReviews#Committers committer] (who may or
 [ContributionReviews#Suggested_patches_from_casual_contributors may not]
 be the actual contributor), `svn commit` can be used to commit the change into
 the
 [http://code.google.com/p/soc/source/browse SoC project SVN repository].

 ==== svn commit messages ====

 Committers should use the template below for `svn commit` log messages:

 {{{
 Log message detailing the change.

 Patch by: Carl Contributor
 Review by: Rick Reviewer
 Review issue: N
 Review URL: http://codereviews.googleopensourceprograms.com/N
 }}}

 `N` is the issue number assigned to the patch by the
 [ContributionReviews#Using_Rietveld_for_code_reviews Rietveld] code
 review tool.

 Note that the person listed with "Patch by:" is the contributor (that
 is, author) of the patch, not necessarily the committer who executes
 the `svn commit` command.  Patches from
 [ContributionReviews#Suggested_patches_from_casual_contributors casual contributors]
 will, by definition, need to be committed by someone other than the
 author.

 === Branches ===

 Occasionally, a major new feature requires changes to many source files.
 In these cases (that are hopefuly rare), it may make sense to do the
 development in a development branch and then `svn merge` the changes back
 into the `/trunk/` once the feature is complete.  While this is a good
 way for a single developer or small team to experiment with major changes
 to the [SoCOverview SoC framework] and
 [MelangeIntro Melange web applications], these sorts of development efforts
 should be started with care.  The effort to successfully merge a major
 overhaul of the existing code can be substantial.

 Given the nature of Python and [AppEngine Google App Engine], it is usually
 possible to do most new feature development in `/trunk/`, adding new modules
 and tests in isolation before a single commit that integrates the new code
 with the existing code that would depend on it.

 == Documentation ==

 === Suggested documentation from casual committers ===

 === Reviews after committing by committers ===

 ----
 _Copyright 2008 Google Inc._
 _This work is licensed under a_
 [http://soc.googlecode.com/svn/wiki/html/licenses/cc-by-attribution-2_5.html Creative Commons Attribution 2.5 License].
 [http://creativecommons.org/licenses/by/2.5/ http://soc.googlecode.com/svn/wiki/html/licenses/cc-by-2_5-88x31.png]
	#summary Guidelines for contributing to Melange
	#labels Contents-Skeleton,Phase-Guidelines

	The [MelangeIntro Melange project] has a variety of opportunities for the
	community to contribute, including:

	* documenting [http://code.google.com/p/soc/w/list?can=2&q=phase:Requirements feature requirements]
	* designing the [http://code.google.com/p/soc/w/list?can=2&q=phase:Design%20design:Model database schema] and different [http://code.google.com/p/soc/w/list?can=2&q=phase:Design%20design:View Views]
	* writing [http://code.google.com/p/soc/w/list?can=2&q=phase:Implementation source code]

	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.

	== Committers ==

	Committers in the Melange community are those individuals who have been
	made project members or project owners in the
	[http://code.google.com/p/soc/ SoC project] on
	[http://code.google.com Google Code]. These individuals have been given
	commit privileges as a result of their previous contributions to the
	Melange community, usually in the form of
	[ContributionReviews#Patches patches]. These individuals are the
	"gatekeepers" of the community, helping to insure that standards such
	as [PythonStyleGuide programming style] and
	[TestingGuidelines testing] that are important to the community as a
	whole are adhered to by each contributor and for each contribution.

	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]

	=== Casual contributors ===

	Casual developers can contribute to the [MelangeIntro Melange project]
	primarily through the [ContributionReviews#Patches patch process]
	described below. Casual contributors still need to agree to the
	[ContributionTerms guidelines for contributors] before their patch
	can be accepted and commmitted by a
	[ContributionReviews#Committer committer].

	== Code reviews ==

	Code reviews are handled in a variety of ways, depending on the size of
	the change and the role of the contributor in the Melange community. Most
	contributions will be in the form of small
	[ContributionReviews#Suggested_patches_from_casual_contributors patches] and
	[ContributionReviews#Reviews_after_committed_patches commits]. Larger
	contributions, such as entire new features, probably require a different
	approach, such as development in a
	[ContributionReviews#Branches development branch]. These have similar
	review approaches, but tailored to the specific situation.

	=== Code changes should be atomic ===

	The commit of a source code change should result in a source repository
	where the software [BuildingMelange builds] and passes any existing
	[TestingGuidelines tests]. So, all related changes need to be committed
	in a single patch, a single commit, or, in the case of branches, a
	single merge.

	=== Patches ===

	The term "patches" is used to refer to relatively small changes to the
	code (and, yes, this definition is intentionally vague). These can either
	be in the form of actual output from the `diff` command, or they can be
	the contents of the commit log message sent to the
	[MailingListsGuidelines#Developer_list developer mailing list] when a
	[ContributionReviews#Committers committer] commits a change using `svn`.

	Patches should be presented for review in the "unified `diff`" format,
	which is the output resulting from the `svn diff` command (the same format
	as that produced by`diff -uNr`, but generated automatically from the
	altered file in the working copy and the original file in the `.svn`
	subdirectory). This is also the same format as the diff contained in the
	commit log messages emailed to the developer mailing list.

	One special note about creating patches: please create the patch from
	the `/trunk/` directory (or the `/wiki/` directory if it is a documentation
	change), so that the reviewers and potential committers can apply the
	patch from the base of their `/trunk/` working copy. For example, for
	a checkout of `https://soc.googlecode.com/svn/trunk/` to `~/svn/soc`,
	create the code review patch with something like:

	{{{
	cd ~/svn/soc
	svn diff soc/models/new_model.py soc/views/deleted_model.py > my_change.diff
	}}}

	In this example, `new_model.py` would be a newly-added module, and
	`deleted_model.py` would be a just-deleted module. The reviewer can
	then apply the patch using something like:

	{{{
	cd ~/svn/soc
	patch -p0 < my_change.diff
	}}}

	The same `/trunk/` directory root approach should be used with
	[http://codereviews.googleopensourceprograms.com/static/upload.py `upload.py`]
	to submit
	[ContributionReviews#Using_Rietveld_for_code_reviews Rietveld code reviews].

	==== Patch emails ====

	Patch emails sent to the developer mailing list should begin with
	`[PATCH]` as the first text in the email subject line, followed by a
	brief (one line) description of the change.

	For "small" patches (for some definition of "small", please use good
	judgment), include the diff in the email body, to facilitate inline
	commenting in the code review email thread. For _all_ patches, including
	both "small" and "non small" ones, include the diff itself as an email
	attachment, to make it easier for reviewers to patch the change into a
	working copy with `patch`. Such a diff attachment can be produced with
	something like:

	{{{
	svn diff some_module.py > some_module.diff
	}}}

	For multiple files changed in the same patch, use something like:

	{{{
	svn diff some_module.py some_other_module.py > my_new_feature.diff
	}}}

	Alternately, committers can use
	[http://codereviews.googleopensourceprograms.com/static/upload.py `upload.py`]
	to submit
	[ContributionReviews#Using_Rietveld_for_code_reviews Rietveld code reviews].

	==== Using Rietveld for code reviews ====

	An instance of
	[http://code.google.com/p/rietveld/ Guido van Rossum's Rietveld]
	code review tool has been set up at
	http://codereviews.googleopensourceprograms.com for use by
	[ContributionReviews#Committers Melange committers]. An account on
	[http://mail.google.com/a/googleopensourceprograms.com googleopensourceprograms.com]
	is required in order to post code review requests and to provide code
	review comments.

	Patches supplied via the Rietveld web form should be created following
	[ContributionReviews#Patches these instructions]. To use the
	[http://codereviews.googleopensourceprograms.com/static/upload.py `upload.py`]
	script provided by Rietveld to start a code review, do something like this
	and respond to the prompts:

	{{{
	cd ~/svn/soc
	upload.py --server=codereviews.googleopensourceprograms.com \
	-- soc/models/new_model.py soc/models_deleted_model.py
	}}}

	Note that a copy of the `upload.py` script can be modified to make the
	default server be `codereviews.googleopensourceprograms.com`, to avoid
	constantly having to supply the `--server` option.

	To upload an updated diff for an existing code review in progress, simply
	supply the extra `--issue=<the existing issue number>` option. Rietveld
	will then display the deltas between the previously supplied diff and the
	new one.

	==== 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] _(not yet enabled at this time)_
	* 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 `svn` repository,
	supplying a suitable commit log message.

	==== 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 `svn` 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"
	message in response (sent back to the list). This does _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.

	=== Other code review possibilities ===

	In the future, tools like
	[http://code.google.com/p/gvn/ Gvn Versions Nicely] might be used for
	"review then commit" code reviews that make use of a "change branch" model.

	=== Committing ===

	Once all of the
	[TestingGuidelines#All_changes_should_pass_smoke_tests_before_review pre-commit tests]
	pass and the [ContributionReviews#Patches patch has been code-reviewed]
	a [ContributionReviews#Committers committer] (who may or
	[ContributionReviews#Suggested_patches_from_casual_contributors may not]
	be the actual contributor), `svn commit` can be used to commit the change into
	the
	[http://code.google.com/p/soc/source/browse SoC project SVN repository].

	==== svn commit messages ====

	Committers should use the template below for `svn commit` log messages:

	{{{
	Log message detailing the change.

	Patch by: Carl Contributor
	Review by: Rick Reviewer
	Review issue: N
	Review URL: http://codereviews.googleopensourceprograms.com/N
	}}}

	`N` is the issue number assigned to the patch by the
	[ContributionReviews#Using_Rietveld_for_code_reviews Rietveld] code
	review tool.

	Note that the person listed with "Patch by:" is the contributor (that
	is, author) of the patch, not necessarily the committer who executes
	the `svn commit` command. Patches from
	[ContributionReviews#Suggested_patches_from_casual_contributors casual contributors]
	will, by definition, need to be committed by someone other than the
	author.

	=== Branches ===

	Occasionally, a major new feature requires changes to many source files.
	In these cases (that are hopefuly rare), it may make sense to do the
	development in a development branch and then `svn merge` the changes back
	into the `/trunk/` once the feature is complete. While this is a good
	way for a single developer or small team to experiment with major changes
	to the [SoCOverview SoC framework] and
	[MelangeIntro Melange web applications], these sorts of development efforts
	should be started with care. The effort to successfully merge a major
	overhaul of the existing code can be substantial.

	Given the nature of Python and [AppEngine Google App Engine], it is usually
	possible to do most new feature development in `/trunk/`, adding new modules
	and tests in isolation before a single commit that integrates the new code
	with the existing code that would depend on it.

	== Documentation ==

	=== Suggested documentation from casual committers ===

	=== Reviews after committing by committers ===

	----
	_Copyright 2008 Google Inc._
	_This work is licensed under a_
	[http://soc.googlecode.com/svn/wiki/html/licenses/cc-by-attribution-2_5.html Creative Commons Attribution 2.5 License].
	[http://creativecommons.org/licenses/by/2.5/ http://soc.googlecode.com/svn/wiki/html/licenses/cc-by-2_5-88x31.png]