One of the greatest things about open source projects is that you have the power to fix bugs, add new features, or improve existing ones.

Since Groovy has moved to Github, this has become even easier to achieve as you can now submit a pull request and link to it from a JIRA report so that it appears in the changelog when a new version is released – Assuming your change is considered valid for inclusion.

This post will go through the steps for getting a new feature into the Groovy codebase.  Or at least the steps required to end up with a push request, as there are no guarantees that the feature I describe will be judged worthy of inclusion.

The Players

To follow this guide, I am assuming a few things

1. You have an account on github
2. You have an account on Groovy’s JIRA
3. You have git installed on your development machine
4. You have set up your ssh keys to access github

Everything here is free to sign up for, or download, so what are you waiting for?  Go do it now, and I’ll wait…

Done?

Good.

The Set-up

So first, we are going to need to generate our own fork of the groovy-core repository on Github.  At the top of the repository page, you should see a block of buttons looking like this:

Just click the “Fork” button, wait for the “Hardcore Forking Action” to complete and you will be taken to your own personal fork of the Groovy project.

We can now get this fork back onto our development machine with git clone:

Moving into our newly created groovy-core directory, we can then set up an upstream remote repository so that we can keep up to date with the main groovy fork.

So your local setup should now look like:

To ensure your master branch is up to date with the main groovy fork, you can simply pull from upstream like so:

It’s worth doing this before working on a feature, as it saves everyone (probably you) work further down the line…

The Tale

So, what are we going to add?  In one of my previous posts, I had to iterate through a list of Closures, and compose them all together like so:

I have also ended up in a situation where I have a list of lists, and I want to find the common element(s) in all of them:

Wouldn’t it be nicer for both of these cases if there was a new form of inject?  One which only took a closure as a parameter, and set the initial value to be the head of the list, and applied the tail of the list to this value?

My two bits of code above would then become:

and

So that’s what we’re going to try and do…

The Wire

Before we start working on our feature, it’s always best to see if it is something already asked for in the JIRA or if it is a bug/feature you got from the JIRA, it’s always worth checking the commit history to see if it is something that’s already been fixed by someone – I’ve tried to fix already fixed bugs more than once, and believe me it’s always worth a check.

So after a quick check, I can’t see anything in the JIRA, so we’ll begin.

First, we’ll create a new branch to keep our code on. That’s simply achieved with:

We can then start writing our test cases to make sure our new code works as expected (and keeps working as expected in future releases).  The tests we’re going to write will live inside src/test/groovy and a quick search of that folder shows us that the file ClosureMethodTest.groovy is a good candidate location for our new tests as that is where the current inject method is tested.

So, lets add a test method to that class, somewhere near the existing testListInject method:

The test method checks the old functionality is working as expected, that our use case works, and then shows what is expected when you have lists containing less than two elements.  EDIT 2012/02/22 In the final pull request – after discussion – calling inject on an empty list or iterable now throws a NoSuchElementException rather than null

So, we can firstly commit this to our local branch

And then we can run the test to make sure it fails as expected

This gives us the following output (showing that there is no method inject that only takes a single Closure parameter):

So we now need to implement our new functionality. At the time of writing this, pretty much all of the extensions to regular Java classes lives in the monolithic DefaultGroovyMethods.java file which can be found in the folder src/main/org/codehaus/groovy/runtime. I believe there is an effort to try and split this file into separate files (see DateGroovyMethods.java, SqlGroovyMethods.java, etc), but we’re going to be adding the the standard DGM class.

So, just above the three argument Collection.inject method (around line 3600 at time of writing), we can add this method

As you can see I have taken the three argument method, removed the initialValue argument then after handling our edge cases simply delegates to the existing method.

One useful thing to notice is the <pre class="groovyTestCase"> block in the javadoc comments.  All of these <pre> blocks are extracted and run by the build tool when it performs a comprehensive test.  It’s a really good way of documenting how functions can be used safe in the knowledge that your examples are correct.

So, lets run our single test again:

Looks good, so we can commit this function to our local branch, and then run the entire test suite to make sure we haven’t inadvertently broken something else.

If you do find that the test build fails, reports can be found and examined in the folder target/test-reports. Have a look what’s broken, fix it, and commit the fix to your local branch.

The Shut-out

So, where are we up to?

We have our new code, on our new branch, and we are ready to send it off to our github fork of groovy-core.

To do this is simple, just issue the command:

So we’ve pushed a new branch to our fork of groovy.  Next, we can create an issue in the Groovy JIRA making sure we fill in the version it affects, and that this is a minor improvement (instead of a bug or new feature)

Then, switch back to github, ensure you are on your branch with the new features

And click the “Pull Request” button in the bar at the top:

On this page, you can fill in the title and description of what your code does (I tend to copy the JIRA entry and add a link back to the JIRA), and check the commits that are going to be added to the request, and when you are ready click “Send pull request”.

And that’s the pull request sent!

And this point, it’s good practice to post the url of your pull request in as a comment on the JIRA issue, as then the source and the issue can be seen no matter which direction you are coming at it.

The Sting

But are we done?  Probably not…

Inject works on Lists, Maps and Iterators.  Would extending it like this be of use to those classes as well? In the JIRA, there’s a request from 2005 that inject should work on arrays, so this is something we should definitely add.  Indeed, the sharp eyed amongst you will notice that I added support for arrays to the pull request in a separate commit.

There’s also the chance that your patch may fail code review.  If this happens, you can address the problems, and re-commit to your branch and the pull request will be updated automatically.

Or of course, your patch may get rejected for causing a breaking change, or having functionality close to something that already exists in the codebase.

But don’t be disheartened! I’ve had many things rejected, but each time I have learned something new about the internals of Groovy or API design.

So what are you waiting for?

@tim_yates