Skip to content

Review Checklist

cpmeister edited this page Jun 24, 2020 · 12 revisions

Although we have coding guidelines and JavaDocs on the APIs, there are mistakes that are regularly done by new contributors in PRs, which often slip through the reviews.

This page should serve as a quick checklist for reviewers to remember what to look for to make sure that those things are addressed by the contributors.

This should be a living document, so every maintainer/reviewer is free to add stuff to the list, which he regularly notices in reviews.

Checklist

  1. proper bundle name in pom.xml ("openHAB Add-ons :: Bundles :: BTicino ... Binding")
  2. include new bundles in build (main pom.xml) and karaf feature (in src/main/feature of the bundle)
  3. NOTICE: EPLv2 license with NOTICE file
  4. NOTICE: all dependencies must be listed in NOTICE file
  5. README: Based on our template, same sections should be present
  6. README: new line after every sentence
  7. README: section headers should be capitalized ("Thing Configuration", not "Thing configuration")
  8. README: mention all thing type ids, channel ids, configuration parameter keys in the appropriate section
  9. Thing/Channel labels should be short (<25 chars, max 2-3 words) and capitalized
  10. conservative use of log levels (mainly debug, unless bugs to report or misconfiguration other than on things)
  11. handler.initialize() to return fast and set a valid/correct Thing status
  12. use of lambdas for runnables
  13. handle REFRESH commands
  14. All conversions of byte[] to String or vice versa should have the Charset specified as well. This includes converting a Input/OuputStream to a Reader/Writer.
  15. Sockets and Input/OutputStreams should be used in try-with-resources statements if possible.
  16. Make sure that @NonNullByDefault added to every class with the exception of classes with a DTO suffix or in a dto package.
  17. Duplicate code should be refactored where possible.
  18. In ThingHandler implementations, all asynchronous futures created during initialize should be cleaned up in dispose.
Clone this wiki locally