Developer Area/Contributing Code

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 irc.freenode.net, or in the Mahara developer forums.

Getting ready for using Gerrit

The Mahara code review system uses Gerrit, a web-based collaborative tool for code revision. This allows anyone who wish to contribute to submit code and get it added to Mahara core. You do not need commit permission on the main Mahara repository, or to be part of the "Mahara Reviewers" group to send your patchsets through the Gerrit system. Essentially, Gerrit is an intermediate step on the way of the patch between your local workspace and the main Mahara repository. For your local git clone of Mahara, Gerrit is just another remote branch which one can push changes to that will be shown on the Gerrit web interface.

Initial setup

First you'll need to create accounts on Launchpad and reviews.mahara.org, and install some local software. You'll find instructions for that here:

• Developer Area/Developer Tools.

Get a copy of the Mahara source code from git

Grab a read-only copy of the Mahara repo from Github:

cd /path/to/your/web/directory
git clone [email protected]:MaharaProject/mahara.git

This will create a copy of the Mahara source code at /path/to/your/web/directory/mahara. For testing purposes, you'll probably want to configure this code so that it actually runs as a copy of Mahara on your local machine. If you need instructions on how to do that, check out Setting up your developer environment.

Setting up Gerrit in your local Mahara repository

Add the gerrit site as a new remote in your local git repository. (Replace username with your username on reviews.mahara.org). The username is case-sensitive, which is why we recommend using a lowercase one.

cd /path/to/your/web/directory/mahara
git remote add gerrit ssh://username@reviews.mahara.org:29418/mahara

If you cannot use SSH, do:

git remote add gerrit https://reviews.mahara.org/mahara

To test whether this has worked correctly, try this command:

git fetch gerrit

You should see git fetch a list of branches. If you don't see that, or you see an error message, check out the troubleshooting your Gerrit connection page.

If that doesn't work and you get a message mentioning diffie-hellman, add the following lines to your ssh config file in your home directory. We need those due to an older version of SSH still being used for the moment:

Host reviews.mahara.org
    Hostname 202.78.242.17
    Port 29418
    KexAlgorithms +diffie-hellman-group1-sha1

Next, you will need to add a commit hook to your local git repo, to generate a Change-Id: line in your commit messages as required for proper gerrit functioning:

scp -p -P 29418 username@reviews.mahara.org:hooks/commit-msg .git/hooks/

Make this file executable:

 chmod u+x /path/to/your/web/directory/mahara/.git/hooks/commit-msg

If you wish to have your instance of Mahara to be able to compile the css (version 15.10+) on the checkout of a patchset from Gerrit then you can, from your Mahara base directory, do the following:

touch .git/hooks/post-checkout && chmod 764 .git/hooks/post-checkout

Edit the file and add the following, making sure the #!/bin/bash is the first line of the file:

#!/bin/bash
 
set -e
 
# The three values we get from post-checkout hook
prevHEAD=$1
newHEAD=$2
checkoutType=$3
 
[[ $checkoutType == 1 ]] && checkoutType='branch' || checkoutType='file' ;
 
newHEADcheck=$(git name-rev --name-only $newHEAD)
branch=$newHEADcheck
if [ $newHEADcheck = 'undefined' ]; then
    # see if we can find out what the branch we will be merging the patch to 
    maharaBranch=$(curl -s https://reviews.mahara.org/changes/?q=$newHEAD | grep branch)
 
    regex='"branch":"([^"]+)"'
    if [[ $maharaBranch =~ $regex ]]; then
        branch=${BASH_REMATCH[1]}
    fi
 
    if [ "$branch" = "main" ]; then
        # get git root directory
        gitRoot=$(git rev-parse --show-toplevel)
        cd $gitRoot
        printf '\npost-checkout hook\n'
        make cleancomposer
        make initcomposerdev
    fi
    make clean-css
    make css
fi

So now when you checkout a gerrit patch that is on main branch it should compile the css.

Preparing / organising your changes

Some basic guidelines:

• 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 makes it harder to review, manage, and merge them. For instance, if it turns out that one of the three fixes is incorrect, while the other two are ready to merge, or if only one fix needs to be back-ported to previous versions of Mahara.
• 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.
• Group related patches together into topics
  • 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.
• Make sure that the new code follows our coding guidelines

Here are some additional articles you might be interested in reading:

• Writing good patches
• Keeping source code history clean
• Creating Clean Patches

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 in the root directory of your feature/bugfix branch, eg the parent directory of the htdocs/ directory:

 make push

or if you want to add a Gerrit tag:

 make push TAG=topic_for_this_change

Note: You only need to do the topic tag once.

If what you are working on is not finished but you want to save / show your workings to others you can push the patch as 'Work in progress' via:

 make wip

or

 make wip TAG=topic_for_this_change

This will run a few initial sanity checks and either push your changes to Gerrit or will return warnings about coding problems that you will need to fix (see below).

Note: Setting the topic is encouraged for patches that contain new functionality so that they can be spotted more easily as someone from the core team needs to do the front-end verification. A tester's OK would not be enough as they may only test that it works, but not review the implementation itself. The topic tag should include "(feature)".

Note: To set a topic with a spaces in it you will need to do place it in double quotes and put a backslash before the space, eg: TAG="My\ new\ patch"

Once a patch is pushed to Gerrit it is run through a host of automated tests and (after a short while) will update the Gerrit review with a pass or fail in the automated Tests (AT) column.

If your patch fails the automated tests you can follow the link provided in the comments section to find out why your patch failed to help debug and fix the issues (see below).

Apple Mac Computers

Apple Mac Devices may have an extra requirement to update the libraries in use. This is especially important if encountering these error messages when running "make push":

xargs: illegal option -- -

sed: illegal option -- r

On an Apple Mac device, the version of "xargs" and "sed" is different and you may see the above errors even if these libraries are already installed. To resolve these issues, install gnu-sed and findutils using the "--with-default-names" option to line them up to the expected Linux counterparts.

Install Homebrew

 ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"

Install required libraries of xmllint, sed, and xargs

 brew install xmlstarlet --with-default-names

 brew install gnu-sed --with-default-names

 brew install findutils --with-default-names

Tips:

Update Git to use the correct email address associated with Gerrit Before making any changes by using the command below with the Gerrit email in use:

 git config user.email "[email protected]"

Be careful if using "sudo" (superuser do) for commands when pushing to Gerrit on an Apple Mac device. The SSH keys are user based, and the sudo command may end up using a different set of SSH keys, resulting in the push being rejected.

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 main branch and your current local branch is bug123456, your command will be:

git push gerrit HEAD:refs/for/main

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/main

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/main/topic_for_this_change

Note: Setting the topic is encouraged for patches that contain new functionality so that they can be spotted more easily as someone from the core team needs to do the front-end verification. A tester's OK would not be enough as they may only test that it works, but not review the implementation itself. The topic tag should include "(feature)".

Security patches

For patches for security issues, use:

make security

or

make security TAG=topic_for_this_change

This will submit your patch into gerrit as a "Private" 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 +/-.

Git for Newbs

If you're new to git, here is a cheat sheet of some of the operations you'll likely need to do.

Creating a new branch for your change

We strongly recommend you create one git branch per change you want to submit. First, make sure you have checked out the "main" branch, and that it is up to date and clean of any changes:

 git checkout main -f
 git reset --hard origin/main
 git pull

Then create the new branch. It is useful to give it a meaningful name like a Launchpad bug number.

 git checkout -b bug123456

Make the changes you want. When you're done, you'll need to group them into a git commit and push that commit to gerrit. First, check what changes you have in your local workspace:

 git status

Next, add any new files that you have added which are not yet tracked by git. You will need to add them by name, for example:

 git add htdocs/view/my_new_view_file.php

Then, you can commit the modified/deleted files with:

 git commit -a

Or if you want to choose interactively which bits of code to add:

 git add -p
 git commit

And finally, use this branch when you push to the code review system (see below).

Making changes in existing commits

It's best to check out the commit from Gerrit itself. There will be a link under the download section with "checkout" highlighted - to copy/paste, eg:

 git fetch https://reviews.mahara.org/mahara refs/changes/57/1234/1 && git checkout FETCH_HEAD

Make the changes that you need and add them ready for committing

 git add -p

Then commit these new changes to the same commit message as before:

 git commit --amend

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:

1. Checkout the branch where your original changes are (see above)
2. Make the required changes
3. 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.
4. 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/main

or

make push

In the Gerrit interface it will appear under the same change as a separate patchset ready for another review.

Ordering commits

If you have pushed some commits to Gerrit but then discover that one of your commits is dependent on another you can adjust the hierarchy of your commits like this.

Checkout the commit that you want to come first/be the parent by getting the checkout code from the review, eg:

 git fetch https://reviews.mahara.org/mahara refs/changes/49/1234/1 && git checkout FETCH_HEAD

Then cherry-pick the second/child commit by selecting 'cherry-pick' before copying the link, eg:

 git fetch https://reviews.mahara.org/mahara refs/changes/50/2345/1 && git cherry-pick FETCH_HEAD

If the child commit merge throws an error, ie you should see lines like: hint: after resolving the conflicts, mark the corrected paths hint: with 'git add <paths>' or 'git rm <paths>' hint: and commit the result with 'git commit'

you will need to fix up the merge conflicts - first go:

 git status

to see which files are causing the merge problems. you should see something like 'Changes to be committed:' listing the files that will merge fine, and 'Unmerged paths:' listing files that will need to be fixed.

Edit the files that need to be fixed and you should see in them some new lines added beginning with <<<<<<< HEAD then some code (part 1), then ======= then some other code (part 2), then >>>>>>> followed by commit id and commit title

What you need to do is to work out which bits of code to keep from part 1 and edit part 2 to suit (or visa versa) delete the <<<<<<<, =======, >>>>>>> lines and any code not needd once you are done.

Save the file and then add the resolved file to the commit by adding the full file directly, eg:

 git add /name/of/file.php

once all the Unmerged paths files are fixed and added then go:

 git commit
 make push

If the child commit has merged cleanly onto the parent one you can then push it back to Gerrit

 make push

And now in Gerrit you will see under the dependencies section that the child patch has a new patchset and that patchset depends on the parent patch/commit.

Committing in some easy condensed steps

First commit

1. Make sure you are on the latest main.

 git checkout main
 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 "main".

 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 "-a" means that all files that have changed will be taken into consideration.

 git commit -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

Useful links

• Gerrit official documentation
• Gerrit Review Primer - useful for getting familiar with review interface.

FAQ

Or rather: Questions a couple of people asked that many others might have. :-)

1.

Q: If I was creating a feature request (not a bug), should I still create a launchpad bug?

A: Yes. Launchpad does not distinguish between bugs and feature requests. We identify features by the status "Wishlist".

2.

Q: How do I link my launchpad bug with the Gerrit change?

A: When you put the bug number in your commit message with "Bug 1234567" or "Bug #1234567" then that will be translated into a link to that bug on Launchpad and also automatically update Launchpad to include a link to the review.

It is recommended to start your commit message with the bug number like so:

 Bug 1234567: Your short commit message here
 
 Second line of the commit message with more detail
 If you have a lot to say, break your commit message
 at about 50 characters so nobody needs to scroll
 horizontally in order to read your commit message. ;-)

3.

Q: Do I need to add some tags to launchpad so people will notice it?

A: No. That is not necessary. Tags will be added during the triage process. It is enough for you to report the issue. The core team will assign the appropriate status and priority amongst other things.

4.

Q: How can I request that a "bug" be backported to an older release?

A: We decide that for Mahara core. Generally, we only backport high importance bugs. Once a bug fix has passed review (and is merged), you can backport it for your older version of Mahara yourself.

If you disagree with not backporting a bug fix, you can provide a reason for actually doing so on the Launchpad item.

5.

Q: What is the typical process for creating a patch?

A:

1. . Create the Launchpad bug.
2. . Add the patch to Gerrit so you can link it to the bug number.
3. . Wait for reply and comment.
4. . Fix things up if needed in the code review system.
5. . It gets merged.

6.

Q: I'm having trouble with make css. How do I avoid SyntaxError: Use of const in strict mode?

A: You need to update your nodejs version. This can be achieved via a) Clear NPM's cache:

sudo npm cache clean -f

b) Install a little helper called 'n'

sudo npm install -g n

c) Install latest stable NodeJS version

sudo n stable