Key

This line was added.

This line was removed.

Formatting was changed.

Contents

Developing and submitting patches

Vetting tickets, and screening and committing patches

Screening process description and figure

See here if you are looking for instructions on submitting a bug report or enhancement request. If you are a Clojure contributor having problems with Clojure changes, submitting patches, the status of a patch, or other questions related to changes to Clojure or the contrib libraries, please ask a question on the clojure-dev Google group.

Thanks for contributing to Clojure!

Glossary

Approval status - the Approval field in a JIRA issue. Clojure/core member - Contributor who has the approval to vet tickets in JIRA. Contributor - Person who has signed the Clojure Contributor Agreement. Defect - A bug in the code base. Enhancement - A new feature in the code base. Issue - A ticket in JIRA. Screener - Contributor who is approved to screen tickets. Task - Work that must be done outside the code base.

Developing and submitting patches to Clojure and Clojure Contrib

If you've got a bug fix or feature you'd like to submit to the Clojure, Clojure Contrib Libraries, ClojureScript or Clojure CLR projects, first - thanks! Before you get too deep into the code, please make sure you review the contributing guidelines -- it's important to submit a Contributor's agreement as detailed on that page. Once it's been received (your name will appear on the contributor's list on that page), you can join the clojure-dev Google group. Anyone with a CA can become a member of our Confluence and JIRA. Just sign up (please use a recognizable name), then ask on the clojure-dev group and I'll bump up your membership level to make edits.

So You Have an Idea...

In order to make sure you are not duplicating effort, fixing a feature, or planning an unwelcome change, please coordinate with the community, by first examining existing issues (clojure's, links to the various contrib libraries' JIRA projects can be found on the Where Did Clojure Contrib Go page), and discussing your patch plan on the clojure-dev Google group. If you are not a contributor, anyone may discuss ideas for changes on the Clojure Google group, to which most or all contributors also belong.

Track it in JIRA

If you've got the go-ahead and there isn't already a ticket for your issue/feature, please create one. Make sure the ticket includes a link to the discussion in the Google Group or IRC, so someone reviewing your ticket can quickly get up to speed. Assign yourself to the ticket so people know you are working on it. Please don't assign things to others without their consent.

Prioritization and Milestones (Committers)

Committers use the prioritization and milestone fields. If you are not the committer for a lib, please don't change the priority or the default milestone assignment.

Development

Once you're ready to craft your code, the first thing you'll need is a clone of the Clojure or appropriate repository. The examples below are for the Clojure project -- for submissions to Clojure itself:

Code Block

language

none

$ git clone git://github.com/clojure/clojure.git
$ cd clojure

Next, create a new branch for yourself:

Code Block

language

none

$ git checkout -b fixbug42
Switched to a new branch "fixbug42"

Now you're ready to get hacking. Make sure relevant doc strings are up to date and that all existing regression tests still pass e.g. for clojure core run:

Code Block

language

none

$ ./antsetup.sh # Only needed once in a new Clojure tree, for ant to work
$ ant

As noted in readme.txt, you will need to run ./antsetup.sh as a one-time setup before running ant. If you want to add new tests, that would be great too. Once you've finished making your changes you need to commit them.

Adding patches

Now you can attach that patch file to the JIRA ticket. In the More Actions menu near the top of the page, select Attach Files. Please read and follow the recommendations below when writing comments about your attached patch. Screeners have limited time available for screening. You are more likely to get your patch approved if you can be as clear as you can, and as efficient with their time as possible.

Include the file name and date of the patch in any comments referring to it. It is possible to match up comments with patches based on the date and time, but it is tedious and error prone.

To get email whenever the ticket is updated, click on the word "Watch" in the top right area of the page. This can help you know when someone else comments on your patch, or creates a new one, etc. Click "Watching" if you want to stop these update emails for that ticket.

If you create a new patch that incorporates one or more earlier ones, please combine them all into one patch file, and indicate in your comments that you have done this (with file names and dates of the patches you are superseding).

If one of your patches becomes superseded by a later one, consider removing your patch to avoid confusion. See the instructions under the heading "Removing Patches" below.

Mark the ticket as having a patch ready for screening by editing the Patch attribute. Click the Edit button near the top left of the page for the ticket. In the next page find the heading "Patch" with a popup menu next to it. Select "Code" or "Code and Test" from that menu, then click the Update button at the bottom of the page. If you do not see an Edit button on the page for the ticket, and you have signed a CA, ask on the developer's email list to be given permission to edit Jira tickets.

Removing patches

To remove a patch (e.g. because it is obsolete), go to the page for the ticket and look for the "Attachments" heading beneath the Description text. Far to the right is a plus sign and a triangle. Click on the triangle and select "Manage Attachments" from the menu. Think carefully on which one you want to delete, and click the trash can icon next to it. Note: Most (or all?) people have permission to remove their own attachments, but not those added by someone else.

Updating stale patches

A stale patch means one that used to apply cleanly to the latest Clojure master version, but due to commits made since the patch was created, it no longer does. In particular, the output of this command:

Code Block

language

none

% git am --keep-cr -s < patch_file

includes 'Patch failed' and 'To restore the original branch and stop patching, run "git am --abort"'. You should do the "git am --abort" to get rid of state of the failed patch attempt left behind by the command above.

"git am" is very "fragile", meaning that if the patch_file was created with one version of the source code, all it takes for the command to fail is a change in any of the lines of context present in the patch file, even if it is not one of the lines being changed by the patch. This is especially common for files containing unit tests, because people usually add new tests at the end of such a file, and so the lines of context before the new test change if two different patches add a new test to the end of the same file.

To apply such a patch, the 'patch' program by Larry Wall is extremely useful. It comes preinstalled with Mac OS X and most Linux distributions. You can easily install it with Cygwin for Windows.

Code Block

language

none

% patch -p1 < patch_file

The output will give you some hints of whether each "hunk" of the patch file succeeded or failed. If they all succeed, then likely the only thing wrong with the patch file is that a few context lines were changed. If any hunks fail, patch creates files ending with ".rej" containing rejected hunks that it did not apply, and you can focus on those as places where the source code likely changed more significantly. A command like this will find them all:

Code Block

language

none

% find . -name '*.rej'

You will need to look at those rejected hunks, perhaps think about them for a bit to see if and how they still apply, and apply them by hand-editing the source code yourself.

When recreating a new git format patch with:

Code Block

language

none

% git format-patch master --stdout > patch_file

it puts your name and the current date near the top of the file. If the only changes that you have made are in the context lines, please keep the original author's credit intact by copying the name and date from the original patch that you started from, then upload that.

If you write unit tests where there were none in the original patch, but didn't otherwise modify the original patch, and you would like your name in the commit log for your work, create a separate patch of test additions with your name on it, leaving the original author's name on the updated patch.

Clojure is Stricter than Contrib

The expectation of stability and performance is much higher for Clojure than for Clojure Contrib. Rich personally reviews every change to Clojure. To save Rich's time, other committers may do the work in git. When this happens, you can see Rich's review process in JIRA, both in the comments and in tickets being moved from 'Screened' to 'Ok' by Rich.

Speculative Changes

If you want to share work-in-progress on some feature you are not yet ready to propose with a ticket, you can post a patch made as above, or a pointer to your repo, to the clojure-dev group.

Vetting tickets, and screening and comitting patches

Vetting Tickets (Clojure/core)

The core team vet tickets, getting them into a state where they can be tested, and screened. Steps are:

Take ownership of a ticket that is status "New" and not currently owned by someone else.

Verify that the issue is reproducible, adding a test case if necessary.

Set the approval to "Vetted."

If the ticket has (or you write) a patch that applies, test it. If it seems to work, set the status to "Test."

If there isn't a unit test and you feel like writing one, please do so.

Release ownership of the ticket.

Screening Tickets (Screeners)

Committing Clojure Patches (Committers)

...

How a Ticket Becomes a Commit

This page describes the overall workflow for how tickets (bugs and enhancement requests) make their way through the JIRA ticketing system and ultimately become part of Clojure, ClojureScript, and ClojureCLR.

The overall process described here has several goals:

Maintain Clojure quality

Fix problems that are important to users

Engage the community in working toward the best possible Clojure

Groups

There are several groups involved in this process with increasing levels of responsibility:

Anyone - anyone can submit a bug or enhancement request to Clojure once you have created a Clojure JIRA account

Contributors - anyone that has signed the contributor agreement can supply patches or work on improving tickets

Screeners - a smaller group of trusted individuals have been granted the ability to move tickets through (some of) the stages of the process, in particular the Triage and Screening activities

BDFL - Rich Hickey is the creator and Benevolent Dictator for Life of what goes into Clojure. Stuart Halloway also has a special level of access and typically commits patches to Clojure.

Ticket fields

There are several important fields on a ticket that jointly determine it's "state" in the workflow below. Some key fields to know about:

The Clojure workflow does not really distinguish between these much other than general open/closed differentiation

Approval- a custom field that is (mostly) how Screeners change the state of a ticket

None - new ticket

Triaged - screener has approved the ticket as worth working on

Prescreened - screener has approved the ticket and screened the patch for review

Vetted - screener and Rich have approved the ticket as worth working on

Screened - screener has approved a ticket's patch for review by Rich

Incomplete - screener has requested improvements to a ticket or patch

Ok - Rich has approved the ticket for inclusion

Patch- qualifies the kind of patch attached

None - no patch

Code - code only, no test

Code and Test - code and test

Fix version

Release X.X - specific targeted release

Backlog - will consider in future release

Resolution- when a ticket is closed, it will have a resolution

Declined - did not accept a ticket for work

Duplicate

Completed

Unresolved

Workflow

The diagram below documents the process used for how tickets make their way through the system. The rounded boxes represent states in the workflow. They have well defined criteria (which sometimes cover multiple fields) such that each of these states can have a report. In general, a single line state indicates the Approval state. If additional fields are in play, they are listed after the state.

The colored blocks represent activities performed by different groups - the colors correspond to the group (Orange = contributors, Blue = screeners, Green = BDFL). Diamonds represent decisions to be made during an activity. Activities are described in more detail below the diagram

Goal: create a high quality ticket and patch for consideration (see sections below)

Actions:

Edit ticket or update patch to address problems or gaps based on comments.

Adding a new patch and changing "Patch" attribute to "Code" or "Code and Test" automatically causes a patch to move from the "Needs Patch" to the "Screenable" list of tickets. However, adding a patch to an incomplete ticket does not. Alex Miller periodically scans Incomplete tickets to see if they appear ready to go back to Screenable, and makes those state changes manually.

I find it safest to do committing from a separate local repository. I have a "clojure" git clone that does not have push permissions for dev and screening, and a separate "clojure-for-commit" checkout for committing. This reduces the chance that my muscle memory will produce a "git push" at the wrong time.

Closing Tickets (Committers)

If you are a committer on a lib, you can screen tickets and close them all in one step. Alternately, you can review tickets screened by others, and then either close them or bounce them back (as in the Screening step above).

While you have the ability to just push your stuff into the repo, please use the tickets systems too. It makes a fine todo system, helps organize bigger milestones, lets others hop in and help or provide input, and provides a place to hang discussions about your changes.

Screening process and figure

Screening Process Described

An issue is created in JIRA that expresses some unit of work that is desired. This could be a defect, an enhancement or a task.

Clojure/core team vets the ticket, and changes the approval status to Vetted, indicating this is a problem that we want to solve. Next a contributor writes a patch to address a defect or enhancement, and attaches the patch to the issue. The contributor should also indicate what kind of patch this is in the ticket. The approval status is then set to test, indicating it is dev complete.

A screener will review the issue and marks the approval status as Screened. This will set the issue to waiting on Rich.

Rich comes along and blesses the issue by setting it the approval status to OK, meaning the code is ready to build & push to git.

When the code has been pushed, the issue is marked as Resolved.

At any point along this path a Screener can mark the issue as Declined, meaning it is not something we are going to address.

Also, at any point a Clojure/core member can mark the approval status as Incomplete. This indicates more information is needed to continue work on the issue. There is also an optional Waiting On? field that should be set to indicate who is expected to provide the additional information