Developer Area/Contributing Code
From Mahara Wiki
< Developer Area
If all else fails...
Admittedly, it can be a little intimidating for a new developer to go through the process described on this page to submit patches to Mahara. If you can't get this working, or don't want to, feel free to simply attach your patch file directly to a bug report in Launchpad. (It's helpful if you can specify the exact version of Mahara your patch applies against.) Or link to a commit on github. Or even paste code directly into the bug report. We welcome all contributions. :)
From there, another member of the Mahara community can push your contribution into Gerrit, using git commit --author="Your Name <[email protected]>" to credit you as the author of the change.
But if you plan on being a regular contributor to Mahara, it's preferable if you can follow the process described below. If you have difficulties, feel free to ask for help on #mahara-dev on freenode.irc.net, or in the Mahara developer forums.
Getting ready for using Gerrit
Mahara code review system uses Gerrit web-based collaborative tool for code revision. 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. Essentially, Gerrit revision system is intermediate step on the way of the patch between the main Mahara repository and your local branch. For your local git clone of Mahara code, Gerrit is just another remote branch which one can push changes to that will be shown on the Gerrit revision web-interface.
Initial setup
Before submitting your code for revision, you need to do some initial set up.
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.
If you need more instructions on how to get your developer environment to work, check out Setting up your developer environment.
Setting up Gerrit in your local Mahara repository
First of all you need to add a new gerrit remote branch to your repo (substitute your gerrit username):
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/
Creating a new branch for your change
We strongly recommend you create one git branch per change you want to submit:
git checkout -b bug123456
then commit your changes there:
git commit -a -s
and finally use this branch when you push to the code review system (see below). Please note; this process has changed several times. Are you using the current best practice?
Preparing / organising your changes
Here are some articles you might be interested in reading:
One patchset per logical change
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.
Include technical details with each change
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.
If you are developing a larger feature as a series of related patches, group them together into a topic (see the section below on how to do this in gerrit). A topic is especially important if the first few patches in the series don't really achieve anything on their own. A reviewer will be able to make sense of an earlier patch by looking forward to later ones within the same topic. The commit message of the earlier patch should explain the plan, and the whole series of patches should be submitted at the same time.
Apply new patch on top of clean checked-out branch
Every new patch should be applied on top of the clean checked-out branch to avoid adding dependencies in error.
Consider the reviewer
Following the guidelines above should help to make your patches easier for the reviewer to read and understand. But also try to remember that the person reviewing your code would probably rather be doing something else, like writing their own code. They don't want to have to review your patch twice, so don't submit patches until they're well-tested, and until you're absolutely sure the code is correct.
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.
Signing off your commits
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. You do need to use your real name (sorry, no pseudonyms or anonymous contributions.)
Submitting commits for review
Preferred method
This is the preferred method of pushing things to Gerrit, but you should understand what it does by reading the manual method as well.
Note: To use this, you will need to install xmllint. On debian/ubuntu based systems, this is in libxml2-utils.
To push your change, simply type this from your feature/bugfix branch:
make push
or if you want to add a Gerrit tag:
make push TAG=topic_for_this_change
This will run a few sanity checks and then push your changes. Note: You only need to do the topic tag once.
Manual method
In order to push your changes for review, just push into the project's magical refs/publish/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/publish/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/publish/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/publish/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 the most convenient way:
git push gerrit HEAD:refs/publish/master
or
make push
In the Gerrit interface it will appear under the same change as a separate patchset ready for another review.
Committing in some easy condensed steps
First commit
1. Make sure you are on the latest master.
git checkout master git pull
2. Make changes to the file(s) that you want to fix in your favorite editor.
3. Check which files you have changed.
git status
4. Review your changes by comparing them to the original content of the file.
git diff
5. Double-check on which branch you are. You should be on "master".
git branch
6. Create a new branch which contains the changed files. You can give the branch any name you like as long as you remember what you changed on it. Using the bug number in the branch name is easy to identify it.
git checkout -b bug[bug number]
7. Double-check that you are now on that new branch and that you still see the changed files.
git branch git status
8. Commit your changes to git. Please read how to write good commit messages. Ideally, the Launchpad bug number appears in the first line of the commit so that it appears in the changelog. If the first line would get too long, put the bug number in the second or last line. It will then be linked back to Launchpad and your patch will be associated to the bug correctly. The "-s" means that you sign-off on your patch and the "-a" means that all files that have changed will be taken into consideration.
git commit -s -a
9. Check your local git log that you committed your change and that the commit message doesn't contain any errors. You can make changes to your commit message if necessary with "git --amend".
git log
10. Push your changes to the review system. Make a note of the URL under which your patch is available for review. You will also see it appear on your Gerrit dashboard.
make push
Re-submit a patch
You may have to make changes to a patch that you submitted based on feedback you receive from the reviewers.
1. Make sure that you are on the branch in which you fixed the bug. If necessary, switch to it with git checkout.
git branch git checkout bug[bugnumber]
2. Make the necessary changes.
3. Check which files you have changed.
git status
4. Review your changes by comparing them to the original content of the file.
git diff
5. Add the file(s) to the commit.
git add [filename] OR git add . (adds all files that have changed)
6. Open the commit message and make changes if necessary. Then save the file. The command given here is for vim.
git commit --amend [in vim] :wq
7. Review that your changes to the commit message made it into the log.
git log
8. Push your changes to the review system
make push
Security patches
For patches for security issues, use:
make security
This will submit your patch into gerrit as a "Draft" submission, which will be only visible to you and to people in gerrit's "Security" group. The downside to this is that the code review system doesn't work automatically for Draft submissions, so verifications and review have to be made as freetext comments, rather than +/-.
Useful links
- Gerrit official documentation
- Gerrit Review Primer - useful for getting familiar with review interface.