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.
To follow this guide, I am assuming a few things
Everything here is free to sign up for, or download, so what are you waiting for? Go do it now, and I’ll wait…
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
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…
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:
So that’s what we’re going to try and do…
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
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
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
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.
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!
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?