Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

Screening Tickets (Screeners)

Periodically, screeners will review tickets whose approval is marked 'Test'.

  • Take ownership of the ticket so that others know you are working on it.
  • Make sure to work on defects first. Enhancements are much lower priority, and should typically begin with a design conversation under design

To apply someone's changes you need to first create a branch:

Code Block
languagenone
$ git checkout -b freds_fixbug42
$ git am --keep-cr -s < 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 note the -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.

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)
  • 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?

After review, if you are happy, you should

  • Set the approval of the ticket to screened
  • If you own the lib in question, you can set approval to accepted, and deploy the change

If you are not happy

  • Add a comment to the ticket, explaining what the issues are
  • Set the approval of the ticket to 'Incomplete'
  • Set the waiting-on of the ticket to the person who created the patch

If you aren't sure

  • Get a second opinion, and note this in the comments
  • Set the waiting-on to Rich or Stu, if appropriate

See Screening Tickets.

Committing Clojure Patches (Committers)

...