- Bugs first, then features. Most users do not edit the issue type or severity fields, so most tickets start marked as bugs, and job 1 is to recategorize them.
- Focus on tickets with code. If there isn't code yet, you will have to write it, and have someone else screen it.
- No new features during Beta. The most a feature request can hope for during Beta is to be assigned the "Approved Backlog" release
- Do the important bugs first. There is no AI query to find these. One obvious criterion for importance is difficulty of workaround. Another is regression. Another is upvoted issues.
- Vote up issues you think important.
Bounce bad tickets as quickly as possible.
Reasons to decline a ticket:
- Change does more than one thing. Break it up, submit individual problems and patches.
- Enhancement belongs in a library instead. Be encouraging and redirect the person to an appropriate Contrib (or propose a new Contrib).
- Does not comprehensively consider the root problem.
The following are required in the ticket. Mark the ticket incomplete if any are missing (or fix the gap if you can):
- Clear description of the problem.
- For defects, a reproducing case in a basic repl (no gists, no external github projects, no jars, etc unless absolutely necessary due to AOT etc).
- For defects, a description of the cause of the problem.
- The approach to fixing the problem (or implementing the enhancement). This should explain (as a narrative) the change in the patch.
- For changes that have a performance impact, a performance analysis. Results should be in a table showing before and after timings. Example code to reproduce the timings should be either linked or included. Specify the Clojure and Java version used to take the timings.
- The name of the patch to consider (unless there is only one patch, and even then, it wouldn't hurt).
- Links to background discussions or other relevant material a screener should read.
The following are required in the patch. Mark the ticket incomplete if any are missing:
- Patch file should end in .patch or .diff.
- Commit message should start with the ticket id and have a brief 1 line description of the change. Optionally, it may have a longer description after that.
- Patch should be in proper format and include tests.
Want to Become a Screener?
Screeners are a subset of contributors who are responsible for moving tickets through a review process, and then funneling tickets to the BDFL.
$ git checkout -b freds_fixbug42 $ git am --keep-cr -s --ignore-whitespace < their-patch-file.diff
- make sure you have a clean local checkout
- no need to check on multiple OSes (unless the ticket is specific to this)
- Please If you are committing i.e. you own the lib, please note the importance of -s above, it is important, as it indicates that you are the one accepting this patch.
- The --keep-cr helps when files being patched contain DOS CR/LF line endings. It seems to be harmless when it isn't needed, but leave it off or use --no-keep-cr if you suspect it is causing issues.
- The --ignore-whitespace helps when the only changes made to master since the patch was created are to whitespace in the context lines. Without this option, some patches will fail to apply. With that option, screeners can help avoid making contributors update patches merely because some whitespace changed in master.
Once you have a working copy, you should take note of the following kinds of things:
- Review the code in a diff tool to make sure that nothing extraneous snuck in
- Make sure compile and tests pass
- Test the functionality from the REPL
- Does the patch accomplish the goals stated in the ticket (and any accompanying discuss in Confluence/email/IRC)
- Is the code in proper style? But remember that Library Coding Standards are guidelines, not fixed laws of the Universe.
- If questions were raised in the discussion, are they answered in the ticket?
- Are you happy with the tests, can you follow what they're testing, is there anything missing?
- Are the tests excessive (introducing dependency on implementation detail)?
- Does the documentation still seem right to you?
- Is the implementation idiomatic (if not, explain why, link to examples...)
- Is the patch performance-sensitive? If so, is the approach appropriate?