Developer Area/Contributing Code: Difference between revisions
From Mahara Wiki
< Developer Area
(Blanked the page) |
No edit summary |
||
Line 1: | Line 1: | ||
==Getting ready for using Gerrit== | |||
[https://reviews.mahara.org/ Mahara code review system] uses [http://code.google.com/p/gerrit Gerrit] as revision tool. This allows anyone who wish to contribute to submit the code for revision and get it added to Mahara core. You do not need main repo commit permission or to be part of the 'mahara reviewers' group to send your patchsets through the Gerrit system. | |||
Before submitting your code for revision, you need to do some initial set up. | |||
====Creating a Gerrit account==== | |||
In order to create your account on the [https://reviews.mahara.org/ Mahara code review system]: | |||
# install the [http://wiki.cacert.org/BrowserClients CAcert root certificate] in your browser | |||
# '''uninstall''' the Cookie Pie Firefox extension if you have it. It will prevent you from logging in. | |||
# click the "Sign In" in the top right corner of the page | |||
# enter your preferred OpenID (we recommend using Launchpad: https://launchpad.net/~username) | |||
# click the "Settings" in the top right corner of the page | |||
# add these details: | |||
#* username (hint: use the one you use on mahara.org, '''all lowercase''') | |||
#* full name | |||
#* email address(es) | |||
#* ssh key (hint: use the one you use on gitorious) | |||
====Doing a fresh Mahara clone==== | |||
If you have not done that already, grab a read-only copy of the [http://gitorious.org/mahara/mahara Mahara repo] from Gitorious: | |||
git clone git://gitorious.org/mahara/mahara.git | |||
This new clone will get rid of old branches and tags that have been moved to [https://gitorious.org/mahara/mahara-museum mahara-museum] and it will ensure that you do not accidentally push anything to the main repo. | |||
====Setting up Gerrit in your local Mahara repository==== | |||
First of all you need to add a new gerrit remote branch to your repo (substitute username with the one you have chosen at the step above): | |||
git remote add gerrit ssh://'''username'''@reviews.mahara.org:29418/mahara | |||
Apparently, '''the username is case-sensitive''', which is why we recommend using a lowercase one. | |||
Also you will need to add a commit message hook to your repo, this will generate <tt>Change-Id:</tt> tags in commit messages which is required for proper gerrit functioning: | |||
scp -p -P 29418 '''username'''@reviews.mahara.org:hooks/commit-msg .git/hooks/ | |||
== Submitting your changes == | |||
A list of changes, like "find this file, then after this line, add this new line", is a nightmare to work with. Patches in this form will ''not'' be accepted. | |||
'''Please send one patchset per logical change you are making'''. If you've fixed three or four bugs in entirely different parts of the system, sending them together as one patchset does not help, especially if later it is discovered that one or two of them are not proper fixes | |||
'''With each change, please send in technical detail about them'''. You should briefly describe what the change intends to do. The ''worst'' possible description you could use is "fixed bug", or possibly "rawr". What we want to see is enough detail about what was wrong, and if not obvious from the patch, why you fixed the problem the way you did. If your description starts getting too long (more than a paragraph), it's probably a sign that you should split up your patches. | |||
'''Make sure your changes "blend in" with the surrounding code.''' By that, we mean make sure you're using four spaces for indentation, the same bracing style the surrounding code uses for if {} etc. Make sure your changes are internationalised using our translation system, and that you throw exceptions the same way the existing code does when errors occur. If your code isn't too much different, we'll probably help you out by fixing it up. But if you're using tabs, weird bracing styles and your own functions for database access, your code is sure to be rejected. Remember to make sure that the new code follows our [[Developer Area/Coding guidelines|coding guidelines]]. | |||
Once your patches are tidy enough for submission, it's time to submit them to the Mahara development team for inclusion. | |||
Patches to the core '''must be signed'''. Signing a patch means that you certify that you own the code, or that you have the right to include it in an open source project licensed under the GNU GPL version 3 or later. The long-form version of this statement is called a [[Developer_Area/Certificate_of_origin|certificate of origin]]. | |||
== Submitting commits for review == | |||
In order to push your changes for review, just push into the project's magical <tt>refs/for/'''remote_branchname'''</tt> ref. If you are pushing to the remote master branch and your current local branch is bug123456, your command will be: | |||
git push gerrit HEAD:refs/for/'''master''' | |||
Once you pushed, your change will be available for review on the [https://reviews.mahara.org/ Mahara code review system]. The single change will be created on review system for each commit you pushed. If you push several commits, gerrit will add dependencies between changes based on the git parent ref. | |||
You may also push a single commit, simply use its hash: | |||
git push gerrit '''38f4f96df''':refs/for/master | |||
If you like, you may specify the topic tag that will be shown in your change details on review system. This tag may be associated with a group of changes if you use it for several commits. To do that, simply append it to the destination ref, e.g. | |||
git push gerrit HEAD:refs/for/master/'''topic_for_this_change''' | |||
== Making changes in existing commits == | |||
The <tt>--amend</tt> flag on <tt>git commit</tt> will allow you to attach changes to a previous commit. This also works with the <tt>-s</tt> flag, if you forgot to sign-off originally. | |||
Another option is commit squashing, below. | |||
When your commit has been reviewed, you may require to make some changes before it will be accepted. The changes you will be doing should be [http://book.git-scm.com/4_interactive_rebasing.html squashed] into the same commit you originally pushed for review. You must ensure that the <tt>Change-Id:</tt> line is preserved the same as in the original commit. This will make Gerrit matching this commit to the original one and appending your changes as a new patchset. The simplest workflow would be: | |||
# Make the required changes | |||
# Commit the changes | |||
#* The commit message does not matter as it will be removed when the commit is squashed, so random text here is fine. | |||
# Use [http://book.git-scm.com/4_interactive_rebasing.html interactive rebasing] to squash the latest commit into the previous commit. | |||
#* An example command for this would be: <tt>git rebase -i HEAD~2</tt> | |||
#* Find the latest commit in the list (it should be the bottom-most commit) and replace the word 'pick' with 'f'. This is short for 'fixup', which will squash the commit into the one before it, and discard the commit message. (This is what we want.) You may also use 's' in which case you will be offered to amend the commit message before squashing (ensure that you leave <tt>Change-Id:</tt> of the commit you are squashing onto). | |||
Now you may push the amended commit to review system: | |||
git push gerrit HEAD:refs/for/master | |||
In the Gerrit interface it will appear under the same change as a separate patchset ready for another review. | |||
== Useful links == | |||
* [https://reviews.mahara.org/Documentation/index.html Gerrit official documentation] | |||
* [http://www.itk.org/Wiki/ITK/Gerrit/ReviewPrimer Gerrit Review Primer] - useful for getting familiar with review interface. | |||
[[Category:Developer Area]] |
Revision as of 16:40, 25 Mayıs 2011
Getting ready for using Gerrit
Mahara code review system uses Gerrit as revision tool. This allows anyone who wish to contribute to submit the code for revision and get it added to Mahara core. You do not need main repo commit permission or to be part of the 'mahara reviewers' group to send your patchsets through the Gerrit system. Before submitting your code for revision, you need to do some initial set up.
Creating a Gerrit account
In order to create your account on the Mahara code review system:
- install the CAcert root certificate in your browser
- uninstall the Cookie Pie Firefox extension if you have it. It will prevent you from logging in.
- click the "Sign In" in the top right corner of the page
- enter your preferred OpenID (we recommend using Launchpad: https://launchpad.net/~username)
- click the "Settings" in the top right corner of the page
- add these details:
- username (hint: use the one you use on mahara.org, all lowercase)
- full name
- email address(es)
- ssh key (hint: use the one you use on gitorious)
Doing a fresh Mahara clone
If you have not done that already, grab a read-only copy of the Mahara repo from Gitorious:
git clone git://gitorious.org/mahara/mahara.git
This new clone will get rid of old branches and tags that have been moved to mahara-museum and it will ensure that you do not accidentally push anything to the main repo.
Setting up Gerrit in your local Mahara repository
First of all you need to add a new gerrit remote branch to your repo (substitute username with the one you have chosen at the step above):
git remote add gerrit ssh://username@reviews.mahara.org:29418/mahara
Apparently, the username is case-sensitive, which is why we recommend using a lowercase one.
Also you will need to add a commit message hook to your repo, this will generate Change-Id: tags in commit messages which is required for proper gerrit functioning:
scp -p -P 29418 username@reviews.mahara.org:hooks/commit-msg .git/hooks/
Submitting your changes
A list of changes, like "find this file, then after this line, add this new line", is a nightmare to work with. Patches in this form will not be accepted.
Please send one patchset per logical change you are making. If you've fixed three or four bugs in entirely different parts of the system, sending them together as one patchset does not help, especially if later it is discovered that one or two of them are not proper fixes
With each change, please send in technical detail about them. You should briefly describe what the change intends to do. The worst possible description you could use is "fixed bug", or possibly "rawr". What we want to see is enough detail about what was wrong, and if not obvious from the patch, why you fixed the problem the way you did. If your description starts getting too long (more than a paragraph), it's probably a sign that you should split up your patches.
Make sure your changes "blend in" with the surrounding code. By that, we mean make sure you're using four spaces for indentation, the same bracing style the surrounding code uses for if {} etc. Make sure your changes are internationalised using our translation system, and that you throw exceptions the same way the existing code does when errors occur. If your code isn't too much different, we'll probably help you out by fixing it up. But if you're using tabs, weird bracing styles and your own functions for database access, your code is sure to be rejected. Remember to make sure that the new code follows our coding guidelines.
Once your patches are tidy enough for submission, it's time to submit them to the Mahara development team for inclusion.
Patches to the core must be signed. Signing a patch means that you certify that you own the code, or that you have the right to include it in an open source project licensed under the GNU GPL version 3 or later. The long-form version of this statement is called a certificate of origin.
Submitting commits for review
In order to push your changes for review, just push into the project's magical refs/for/remote_branchname ref. If you are pushing to the remote master branch and your current local branch is bug123456, your command will be:
git push gerrit HEAD:refs/for/master
Once you pushed, your change will be available for review on the Mahara code review system. The single change will be created on review system for each commit you pushed. If you push several commits, gerrit will add dependencies between changes based on the git parent ref.
You may also push a single commit, simply use its hash:
git push gerrit 38f4f96df:refs/for/master
If you like, you may specify the topic tag that will be shown in your change details on review system. This tag may be associated with a group of changes if you use it for several commits. To do that, simply append it to the destination ref, e.g.
git push gerrit HEAD:refs/for/master/topic_for_this_change
Making changes in existing commits
The --amend flag on git commit will allow you to attach changes to a previous commit. This also works with the -s flag, if you forgot to sign-off originally.
Another option is commit squashing, below.
When your commit has been reviewed, you may require to make some changes before it will be accepted. The changes you will be doing should be squashed into the same commit you originally pushed for review. You must ensure that the Change-Id: line is preserved the same as in the original commit. This will make Gerrit matching this commit to the original one and appending your changes as a new patchset. The simplest workflow would be:
- Make the required changes
- Commit the changes
- The commit message does not matter as it will be removed when the commit is squashed, so random text here is fine.
- Use interactive rebasing to squash the latest commit into the previous commit.
- An example command for this would be: git rebase -i HEAD~2
- Find the latest commit in the list (it should be the bottom-most commit) and replace the word 'pick' with 'f'. This is short for 'fixup', which will squash the commit into the one before it, and discard the commit message. (This is what we want.) You may also use 's' in which case you will be offered to amend the commit message before squashing (ensure that you leave Change-Id: of the commit you are squashing onto).
Now you may push the amended commit to review system:
git push gerrit HEAD:refs/for/master
In the Gerrit interface it will appear under the same change as a separate patchset ready for another review.
Useful links
- Gerrit official documentation
- Gerrit Review Primer - useful for getting familiar with review interface.