Development Team/Code Review: Difference between revisions

Line 1:

~~Send~~ your patches to the Sugar [http://lists.laptop.org/listinfo/sugar mailing list].

= Review Process =

We are using email for the review process. In the future we might change to [http://www.review-board.org/ review board] which we are currently investigating. In order to ask for a review of your code please send your patches to the Sugar [http://lists.laptop.org/listinfo/sugar mailing list].

== Process guidelines ==

Break your projects into small pieces that can be submitted for review independently. Submit for review whenever you think you need a review - you can tell that best. The reviewer will use review+ or r+ whenever the patch is accepted and ready to go into git master.

The workflow should be something like:

* code a small part

* submit to review

* push to master

* repeat

There will be cases when this will not be possible but we want to try to follow this guidelines.

== Email guidelines ==

In order to make it easy for the reviewer please include in your email:

* prefix your email title with [PATCH]

* try to make in the title clear what the patch is about

* note which module is effected e.g. sugar, sugar-toolkit...

* note possible dependencies e.g. the patch is for sugar but depend on the current HEAD of sugar-toolkit which went in 5 seconds ago

* note how the patch can be verified / a use case

== Patch guidelines ==

Please try to use [http://www.logilab.org/857 pylint] to verify your patch for things like exceeding 80 columns etc, unused imports and unused variables. Pylint is by far a tool you can 100% rely on but it helps to follow some guidelines and to avoid the most stupid errors like typos. Here is a pylintrc you can use: [http://dev.laptop.org/git?p=sugar-jhbuild;a=blob_plain;f=scripts/data/pylintrc;h=0f911ed7c25050fb3afefb8fbfc0b37af5882fc8 pylintrc]

In the sugar packages use 'make distcheck' to make sure all files are included and the POTFILES.in is up to date.

=== Create a patch ===

Normally you can get a patch from git with:

git diff

If you already committed part or all of the new code you need to specify the id of the last commit before yours:

git diff 34a4876f93894309f77b00281b5cb1bb72b3a1e4

Or if the commit you want the diff of was the most-recent commit, you can do:

git diff HEAD^!

== Reviewer guidelines ==

Please review the overall design of the patch. In many cases a detailed review is welcome as well. If the submitter added a use case for his patch please try that one as well. Use review+ or r+ to mark a patch ready to go into git master.

== Push the patch ==

Patches with a "review+" keyword can be pushed to repository. If you have write access to the sugar repository you can just do it yourself.

If you don't have write access to the repository commit the patch to your local repository.

git commit

Generate a patch set using git. The following command will create one file per commit in the current directory.

git-format-patch origin

== Remind the maintainers about patches ==

If you don't get any response from the maintainers about the patches you submitted in a few days, feel free to remind about them using the Sugar mailing list or IRC channel.

@@ Line 1: / Line 1: @@
-Send your patches to the Sugar [http://lists.laptop.org/listinfo/sugar mailing list].
+= Review Process =
+We are using email for the review process. In the future we might change to [http://www.review-board.org/ review board] which we are currently investigating. In order to ask for a review of your code please send your patches to the Sugar  [http://lists.laptop.org/listinfo/sugar mailing list].
+== Process guidelines ==
+Break your projects into small pieces that can be submitted for review independently. Submit for review whenever you think you need a review - you can tell that best. The reviewer will use review+ or r+ whenever the patch is accepted and ready to go into git master.
+The workflow should be something like:
+* code a small part
+* submit to review
+* push to master
+* repeat
+There will be cases when this will not be possible but we want to try to follow this guidelines.
+== Email guidelines ==
+In order to make it easy for the reviewer please include in your email:
+* prefix your email title with [PATCH]
+* try to make in the title clear what the patch is about
+* note which module is effected e.g. sugar, sugar-toolkit...
+* note possible dependencies e.g. the patch is for sugar but depend on the current HEAD of sugar-toolkit which went in 5 seconds ago
+* note how the patch can be verified / a use case
+== Patch guidelines ==
+Please try to use [http://www.logilab.org/857 pylint] to verify your patch for things like exceeding 80 columns etc, unused imports and unused variables. Pylint is by far a tool you can 100% rely on but it helps to follow some guidelines and to avoid the most stupid errors like typos. Here is a pylintrc you can use: [http://dev.laptop.org/git?p=sugar-jhbuild;a=blob_plain;f=scripts/data/pylintrc;h=0f911ed7c25050fb3afefb8fbfc0b37af5882fc8 pylintrc]
+In the sugar packages use 'make distcheck' to make sure all files are included and the POTFILES.in is up to date.
+=== Create a patch ===
+Normally you can get a patch from git with:
+ git diff
+If you already committed part or all of the new code you need to specify the id of the last commit before yours:
+ git diff 34a4876f93894309f77b00281b5cb1bb72b3a1e4
+Or if the commit you want the diff of was the most-recent commit, you can do:
+ git diff HEAD^!
+== Reviewer guidelines ==
+Please review the overall design of the patch. In many cases a detailed review is welcome as well. If the submitter added a use case for his patch please try that one as well. Use review+ or r+ to mark a patch ready to go into git master.
+== Push the patch ==
+Patches with a "review+" keyword can be pushed to repository. If you have write access to the sugar repository you can just do it yourself.
+If you don't have write access to the repository commit the patch to your local repository.
+ git commit
+Generate a patch set using git. The following command will create one file per commit in the current directory.
+ git-format-patch origin
+== Remind the maintainers about patches ==
+If you don't get any response from the maintainers about the patches you submitted in a few days, feel free to remind about them using the Sugar mailing list or IRC channel.