Review Checklist

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.
19. Thing config parameters: Specify units where applicable (e.g. unit="s" for seconds)
20. Thing config parameters: Add min/max value where applicable
21. Thing config parameters: Add context tag where applicable (e.g. <context>network-address</context>)
22. Channels declaration: Use Units of Measure (e.g. Number:Temperature)
23. Non-static fields and variables: Use camel case (i.e. no underscores or prefixes in names)
24. Use primitive over complex types (e.g. int vs. Integer)
25. Log stack traces only on severe errors (e.g. detection of a bug)
26. Don't log if a Thing goes offline due to an error. The text passed to updateStatus() is logged by the framework.
27. Use checked exceptions: Extend custom exceptions from Exception
28. Don't throw RuntimeException on expected errors
29. Cache result of getConfigAs() and getConfiguration()
30. When creating a thread, declare it as daemon: Thread.setDaemon(true)
31. Name createad threads with Thread.setName() or via Thread's constructor
32. Specify representation property in discovery results
33. Cancelling a task (Future): There's no need to check if it have been cancelled already
34. Any compiler warning that can't be avoided should be annotated with @SuppressWarnings
35. Check checkstyle warnings in target/code-analysis/report.html
36. Check formatting issues with mvn spotless:check -Dspotless.check.skip=false