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
There are several groups involved in this process with increasing levels of capability:
- 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 or otherwise 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.
There are several important fields on a ticket that jointly determine it's "state" in the workflow below. Some key fields to know about:
- JIRA status- these govern the default JIRA workflow and consist of Open, In progress, Reopened, Resolved, Closed
- 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
- 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
- Not Approved - ?? deprecated?
- Test - ?? deprecated?
- Patch- qualifies the kind of patch attached
- None - no patch
- Code - code only, no test
- Code and Test - code and test
- Fixed, New, Invalid, Accepted - ?? deprecated?
- Fix version
- Release X.X - specific targeted release
- Backlog - will consider in future release
- Approved Backlog, Reviewed Backlog - ?? deprecated?
- Resolution- when a ticket is closed, it will have a resolution
- Declined - did not accept a ticket for work
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
- Who: Screeners
- Report: Open tickets
- Goal: decide whether the bug or enhancement described in the ticket is actually a real bug or enhancement.
- For bugs, there should be some demonstration that the problem actually exists (output from a repl, test, etc).
- Comment on ticket to ask for more information, better description, better demonstration of problem, etc
- Close with Resolution=Decline, reasons:
- Not a bug: submitter misunderstood or misused a feature
- Scope too big: feature may be better served by creating a page in the design wiki than in a ticket
- Enhancement not wanted: enhancement is not something we want to do
- Duplicate: of existing ticket
- Set Approval=Triaged - problem is ok
- If needed, adjust ticket to standards of triaged ticket below
- Who: Rich
- Report: Triaged tickets
- Goal: second check on whether the bug/enhancement is worth working on and decision of whether it's suitable for the next release.
- Close w Resolution=Declined - as above, ticket may not be something we want to address
- Set Approval=Vetted - problem is good
- Who: Rich
- Report: Vetted tickets
- Goal: determine whether a ticket is in scope for next release or should be in backlog
- Set Fix Version to "Backlog" - don't want to fix it in the next release
- Set Fix Version to current release
- If does not have patch, will show up in "Needs Patch" report
- If does have patch, will show up in "Screenable" report
- Dev Patch
- Who: contributors (anyone with signed CA)
- Report: "Needs Patch" and "Incomplete" tickets
- Goal: create a high quality ticket and patch for consideration (see sections below)
- Edit ticket or update patch to address problems or gaps based on comments.
- Who: Screeners
- Reports: Screenable tickets (for new vetted tickets with patches), Incomplete tickets that have changed recently and may need re-review
- Goal: verify that ticket and patch are ready for Rich to review.
- The quality bar is HIGH - the ticket and patch should be perfect
- Set Approval=Incomplete and add comment describing needed improvements
- Set Approval=Screened - ticket and patch are perfect and Rich should review
- Who: Rich
- Report: Screened tickets
- Goal: Rich blessing the change
- Set Approval=Incomplete - ticket or patch needs improvement
- Set Approval=OK - everything is good, ready to commit
- Who: Stu H (usually)
- Report: OK tickets
- Goal: Final review of change and commit to Clojure source
- Set Approval=Accepted and commit patch to clojure src
- Who: Rich (primarily)
- Report: Backlog tickets
- Goal: See if backlogged tickets should be pulled into next release
- Set Fix Version from Backlog to current release
- (or don't to leave in Backlog)
See Submitting bugs and enhancement requests 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!
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.
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:
Next, create a new branch for yourself:
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:
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.
Now that you've made your changes it's time to get them into a patch. You need to update the repo and fix any conflicts you had.
Once you've fixed any conflicts, you're ready to create a patch:
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.
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:
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.
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:
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:
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.
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)
See Screening Tickets.
Committing Clojure Patches (Committers)
- start with a ticket that has been OKed by Rich
- make sure you have the right patch
- make sure the author has signed the CA
- steps that can be batched up:
- double-check that the patch applies cleanly and builds locally
- commit and push the patch
- close the associated ticket(s), if any
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
Some useful filters
Incomplete Issues: http://dev.clojure.org/jira/secure/IssueNavigator.jspa?mode=hide&requestId=10011
See Also http://clojure.org/patches
Figure of JIRA state changes