Wikidata/Development/Process

This page describes the workflow for developing features for the Wikibase extension.

General overview

We have considerably simplified the development process. Basically, whenever you develop something — develop a complete new feature or just make a small fix — you create a new branch. Whenever it makes sense, you squash the changes into one commit, rebase it on the current master, and submit that commit for review on Gerrit.

Once the changes are on gerrit, they need to be reviewed and approved for merging by another developer.

So, to keep things moving, you should review at least as many changes as you submit for review!

If possible, try to follow mw:Git/Commit message guidelines.

Making Changes

As a general rule, we'll try to follow the workflow established by the Wikimedia Foundation, as documented on mw:Git/Workflow#How_to_submit_a_patch.

For a smallish change (basically, anything that you would develop by yourself on a single day), use the following workflow:

Preparing your Patch

Make sure you are on "master" and create the branch from there

git status
# On branch master

Create your feature branch locally

git checkout -b feature_branch_xyzzy

Now you can work on that branch until your feature is ready for review. Note the gerrit likes to have a single commit on your feature branch and will complain that there are more. So, if you have already committed to the branch and want to change something later, use git commit --amend to amend your original commit instead of creating a new one.

While you are working on your patch, make sure to add unit tests for any new classes or functions to introduce, and to update unit tests in places where you modify existing code. Run the Wikibase test group fequently while you are working on the code:

 cd tests/phpunit
 php phpunit.php --group Wikibase

Before submitting your patch for review, test it thoroughly. It should apply cleanly to master, and it should pass all unit tests.

HINT: committed to the wrong branch?

It sometimes happens that you started to work on a change, and committed it, but forget to create a feature branch first. So you end up committing to master (or some other branch you were working on). This can easily be fixed.

First, write down the id (the hash) of your commit. It's shown when you commit, and you can see it using git log.

Then, reset the "wrong" branch, removing the commit from it:

 git reset --hard HEAD^

Note: for two commits, use HEAD^^ or HEAD{2}

Note: your commit is now "detached" from any branch. You can still find it using git reflog.

Now, create your feature branch as described above:

 git checkout master
 git checkout -b feature-thingy

...and finally apply your commit to the new branch:

 git cherry-pick <commit-id>

that's it, your commit is now on the feature branch and no longer on the other branch, and you are ready to submit it for review!

Submitting your Patch

So, rebase to the latest master, and resolve any conflicts:

 git rebase origin/master

Now, run all mediawiki unit test to make sure your changes didn't break anything (yes, even if your were only working on an extension). Make sure you have the extension in question enabled in your LocalSettings.php. Then run:

 cd tests/phpunit
 php phpunit.php --group Wikibase

If this does not report any errors, you are now good to submit the test for review.

Caveat: if phpunit reports timeout errors, open tests/phpunit/suite.xml and change the "strict" setting to false. Make sure to not commit this change, though.

In order to submit your patch for review, first commit it to your branch, if you havn't already. Make sure to use a descriptive commit message. The first line of the commit message should by itself already describe the change, it will be used for the subject line of emails, etc. So, commit the change (using --amend if it's not your first commit on the branch). Make sure you first call git add for all files you modified or added:

 git add foo.php
 git add bar.php
 git commit

Caveat: commit -a will automatically include all modified files, but not new files!

Finally, send the change to gerrit for review:

 git review

You need to have git-review installed for this (see mw:Git/Workflow#git-review for instructions). And for every repository you want to use git-review with, run git review -s once, to set it up for that repository.

Fixing your Patch

Often, reviewers will complain about some aspect of your patch, and ask you to improve it. To do that, go back to your feature branch:

 git checkout feature_branch_xyzzy

or, if you don't have that around, fetch the change from gerrit

 git review -d <number>

This will automatically create the appropriate branch.

Now you can fix whatever the reviewers complained about. When you are done, amend your original commit (do not create a new commit). Again, don't forget to git add anythign you changed:

 git add foo.php
 git add bar.php
 git commit --amend

Please add a note about your changes at the bottom of your original commit message, but above the Change-id line. That line is magic automatically injected by git-review and must always remain the last line in the commit message!

Once you have done this, send the patch to gerrit again, but rember to use the -R option with git-review, so you don't clutter your original change with merges caused by a rebase:

 git review -R

Repeat, until a reviewer has approved, verified and merged your change (see below).

Collaboration and Dependent Changes

In some cases, you may want to work on a feature that depends on some other change that was already submitted for review, but that has not been merged into master yet. You can do that by simply downloading that change (which will put you on the corresponding feature branch automatically), make your change and submit it as normal. Gerrit will detect that your change was done on a feature branch, and depends on the earlier change. That dependency will also be visible on Gerrits web interface.

This kind of dependency allows for multiple developers to collaborate on a feature, building changes on top of each others changes, without having to wait for each change to be reviewed and merged.

So, for example, let's say you have the following situation:

You want to add feature Y, which depends on feature X by Hans which was submitted as change 1234, but has not yet been merged into master.

So, you donwload that change:

 git review -d 1234

This will put you on the respective feature branch, which will look something like:

 review/hans/feature-X

Now, make your changes as normal, and finally commit and push them for review:

 git commit
 git review

Git will create a new change, say 1245, and mark it as depending on 1234. It will not be possible to merge 1245 before 1234 is merged.

Note: git review will warn that you are about to submit multiple changes when you try to submit a dependent change. You can ignore this, as long as all of the changes in the list have already been submitted for review, or you actually want to submit all of them for review at once. Would be nice if gerrit would tell us which of the commits on the branch were already uploaded as changes...

Amending Changes Other Changes Depend On

Say you have two changes (let's call them A and B, with the commit hashes A1 and B1) where B depends on A, as described above (so, A1 is B1's parent commit). Now A needs to be amended for some reason, creating A2. In this situation, B becomes obsolete, because B1 depends on the obsolete commit A1. To fix this, do the following:

1. download change B: git review -d B, where B is B's change ID.
2. rebase B1 on A2, creating B2: git rebase A2', where A2 is A2's commit hash.
   1. resolve any conflicts, use git rebase --continue to continue the rebase.

   Hint: if git rebase tells you that there are no changes after you resolved some conflicts, use git rebase --skip to resolve this.

3. use git log to verify that the last commit in the log is still B, and the commit message still contains B's change ID.

Now the situation is resolved as far as git is concerned, but we still have to tell gerrit. So now we submit B2 to gerrit for review: git review -R

Gerrit should now figure out what you have done, showing a new change set B2 for B, and mark B as no longer obsolete, but still depending on A.

If you have more changes (say, C, D, and E) that depend on B, you have to repeat the procedure rebaseing C on B, D on C, and so on.

Workflow for Reviewing Changes

There are two parts to reviewing a change: approval and verification. Approval is conceptual and formal agreement, while verification is about technical compliance.

Approval

To approve a change, visit it's page on gerrit and look through the diffs (note the convenient "next" and "previous" links on the diff pages). Try to understand the intend and implementation of the change, and check the coding style.

The most important part of course is that the new code is sensible conceptually and that it is implemented correctly. But there are some more things that should be considered:

• is the code internationalized? Are system messages used where they should be?
• does the code follow the code conventions with respect to naming, indentation, spacing, etc?
• is the code thoroughly documented on the class and method level?
• does the code come with the appropriate unit (and/or integration) tests?
• Is the code efficient, does it make use of caching where appropriate?

If you have questions or find anything objectionable, click inside the diff to leave an inline comment. Note that your inline comments will be saved as drafts but not directly published - this also applied to replies to comments! You need to actually post a review verdict (below) to publish your comments.

It's also useful to actually try the code you are reviewing. To do that, first download the change into a local branch:

 git review -d <number>

...where <number> is the (decimal) change number from the gerrit URL or the (hex) change id given on the gerrit page. This will create a local branch for the change set.

Now, try the new functionality (or whatever the change provides) at least superficially.

After you are finished your review, you should post your verdict by pressing the review button. Select the appropriate level of approval (between -2 and +2), add a comment if you like, and hit "Publish Comments". If you just want to publish your inline comments (or replies), leave the rating at 0.

Unless you have also done verification, leave the verification section untouched.

If you think the change can be merged, proceed to the merging section.

Verification

Verification makes sure that the change is technically complied, that is, it

1. applies cleanly to the latest master
2. passes all tests it provides
3. causes no other tests to fail

This verification would ideally done automatically by Jenkins (see our request on Bugzilla). For the moment however, we are stuck doing this manually. So, here are the steps:

First, download the change into a local branch

 git review -d <number>

...where <number> is the (decimal) change number from the gerrit URL or the (hex) change id given on the gerrit page. This will create a local branch for the change set.

Next, try to merge the latest version of the master branch into the change's branch by doing:

 git pull origin master

Note: technically, we want to check whether the change can be merged into master, not vice versa. But since 3-way-merge is a commutative operation, the resulting source code will be the same either way.

If there are conflicts, the change should be rejected (set to -1) as broken.

Run PHPUnit tests

The the merge was successful, next try running the Wikibase unit tests:

 cd tests/phpunit
 php phpunit.php --group Wikibase

This is just for finding any obvious breakage early on. If this passes, try running all unit tests:

   php phpunit.php

If any tests fail, the change should be rejected (set to -1) as broken.

To get rid of failing tests outside our groups, for example all kinds of failures from testing with SqLite which probably are not common here, try --exclude-group sqlite

   php phpunit.php --exclude-group sqlite

Run QUnit tests

As of now, we only have QUnit tests for the Wikibase repo extension. Let them run by pointing your browser to the following URL:

   http://localhost/<path to your MediaWiki installation>/index.php/Special:JavaScriptTest/qunit?filter=wikibase

Having done changes to core, you may rather run all QUnit tests by not applying a filter:

   http://localhost/<path to your MediaWiki installation>/index.php/Special:JavaScriptTest/qunit

Run Selenium tests

TBD: Selenium tests.

Post your verdict

To post your verdict, press the review button on the change's page on gerrit. Select the appropriate level of approval (-1 if it failed or +1 if all is well), add a comment if you like, and hit "Publish Comments". Leave the approval rating as it is, unless you also went through the approval process.

If you think the change can be merged, proceed to the merging section.

Merging

Once the change is approved (+2) and verified (+1), it can be merged in to the main line's master branch. To do so, click the submit change button on the change's gerrit page. Gerrit will then post a message containing the result of the merge attempt. Either the change gets merged, or the merge failed for some reason.

If the merge failed, please use the Review button to change the verified level to -1 (broken).