Coding Guidelines

Scala
- Use vals when possible
- Use private when possible
- Method and member variable names should be in camel case with an initial lower case character like aMethodName
- Constants should be camel case with an initial capital LikeThis not LIKE_THIS
- Prefer a single top-level class per file for ease of finding things
- Do not use semi-colons unless required
- Avoid getters and setters
- Optional parenthesis for no-arg methods--use them if the method has a side-effect, otherwise omit them. For example fileChannel.force() and fileChannel.size.

Logging
- Logging is one third of our "UI" and it should be taken seriously. Please take the time to assess the logs when making a change to ensure that the important things are getting logged and there is no junk there.
- Logging statements should be complete sentences with proper capitalization that are written to be read by a person not necessarily familiar with the source code. It is fine to put in hacky little logging statements when debugging, but either clean them up or remove them before checking in. So logging something like "entering SyncProducer send()" is not appropriate.
- Logging should not mention class names or internal variables
- There are four levels of logging TRACE, DEBUG, INFO, WARN, ERROR, and FATAL, they should be used as follows.
   - INFO is the level you should assume the software will be run in. INFO messages are things which are not bad but which the user will definitely want to know about every time they occur.
   - TRACE and DEBUG are both things you turn on when something is wrong and you want to figure out what is going on. DEBUG should not be so fine grained that it will seriously effect the performance of the server. TRACE can be anything. Both DEBUG and TRACE statements should be wrapped in an if(logger.isDebugEnabled) check to avoid pasting together big strings all the time.
   - WARN and ERROR indicate something that is bad. Use WARN if you aren't totally sure it is bad, and ERROR if you are.
   - Use FATAL only right before calling System.exit()

Monitoring
- Monitoring is the second third of our "UI" and it should also be taken seriously
- 

Unit Tests
- Be aware of the methods in TestUtils that make a lot of the below things easier
- Unit tests should test the least amount of code possible, don't start the whole server unless there is no other way to test a single class or small group of classes in isolation
- Tests should not depend on any external resources, they need to set up and tear down their own stuff. This means if you want zookeeper it needs to be started and stopped, you can't depend on it already being there. Likewise if you need a file with some data in it, you need to write it in the beginning of the test and delete it (pass or fail).
- It is okay to use the filesystem and network in tests since that is our business but you need to clean up after yourself. There are helpers for this in TestUtils.
- Do not use sleep in tests, it is always wrong. Write tests and APIs in such a way that they are not timing dependent. Seriously.
- It must be possible to run the tests in parallel, without having them collide. This is a practical thing to allow multiple branches to CI on a single CI server. This means you can't hard code directories or ports or things like that. Again TestUtils has helpers for this stuff.

Configuration
- Configuration is the final third of our "UI"
- Names should be thought through from the point of view of the person using the config

Concurrency
- Encapsulate synchronization when possible. That is, locks should be private member variables within a class and only one class or method should need to be examined to verify the correctness of the synchronization strategy
- Annotate things as @threadsafe when they are supposed to be and @notthreadsafe when they aren't to help track this stuff
- There are a number of gotchas with threadpools: is the daemon flag set appropriately for your threads? are your threads being named in a way that will distinguish their purpose in a thread dump?

Backwards Compatibility
- Our policy is that we always support backwards compatibility for one release to enable no-downtime upgrades. This means the server MUST be able to support requests from both old and new clients simultaneously. The code handling the "old" path can be removed in the next release. A typical upgrade sequence would be server, consumers, clients.
- There are three things which require this binary compatibility: request objects, persistent data structure (messages and message sets), and zookeeper structures and protocols. The message binary structure has a "magic" byte to allow it to be evolved, this number should be incremented when the format is changed and the number can be checked to apply the right logic and fill in defaults appropriately. Network requests have a request id which serve a similar purpose, any change to a request object must be accompanied by a change in the request id. Any change here should be accompanied by compatibility tests that save requests or messages in the old format as a binary file which is tested for compatibility with the new code.