Inspired by https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md.
We format commit messages to adhere to the conventional commit standard. The commit messages are also used to automatically create and version releases using semantic-release.
Pull requests are usually merged into master using the rebase and merge strategy.
A typical pull request should strive to contain a single logical change (but not necessarily a single commit). Unrelated changes should generally be extracted into their own PRs.
If a pull request contains a stack of more than one commit, then popping any number of commits from the top of the stack, should not break the PR, ie. every commit should build and pass all tests.
Commit messages and history are important as well, because they are used by other developers to keep track of the motivation behind changes. Keep logical diffs grouped together in separate commits and order commits in a way that explains by itself the evolution of the change. Rewriting and reordering commits is a natural part of the review process. Mechanical changes like refactoring, renaming, removing duplication, extracting helper methods, static imports should be kept separated from logical and functional changes like adding a new feature or modifying code behaviour. This makes reviewing the code much easier and reduces the chance of introducing unintended changes in behavior.
Whenever in doubt on splitting a change into a separate commit, ask yourself the following question: if all other work in the PR needs to be reverted after merging to master for some objective reason (eg. a bug has been discovered), is it worth keeping that commit still in master.
We recommend you use IntelliJ as your IDE. The code style used is the Google Java style. It is automatically enforced using spotless.
To run spotless and other checks before opening a PR: ./gradlew :check
In addition to those you should also adhere to the following:
The purpose of code style rules is to maintain code readability and developer efficiency when working with the code. All the code style rules explained below are good guidelines to follow but there may be exceptional situations where we purposefully depart from them. When readability and code style rule are at odds, the readability is more important.
Keep code consistent with surrounding code where possible.
Alphabetize sections in the documentation source files (both in the table of contents files and other regular documentation files).
When appropriate, use the stream API. However, note that the stream implementation does not perform well so avoid using it in inner loops or otherwise performance sensitive sections.
Consider using String formatting (printf style formatting using the Java
Formatter class): format("Session property %s is invalid: %s", name, value)
(note that format() should always be statically imported). Sometimes, if you
only need to append something, consider using the + operator. Please avoid
format() or concatenation in performance critical sections of code.
Avoid using the ternary operator except for trivial expressions.
It is suggested to declare members in private inner classes as public if they are part of the class API.
Prefer AssertJ for complex assertions.
Maintain the same quality for production and test code.
Please avoid abbreviations, slang or inside jokes as this makes harder for
non-native english speaker to understand the code. Very well known
abbreviations like max or min and ones already very commonly used across
the code base like ttl are allowed and encouraged.
Avoid using the default clause when the switch statement is meant to cover all
the enum values. Handling the unknown option case after the switch statement
allows static code analysis tools (e.g. Error Prone's MissingCasesInEnumSwitch
check) report a problem when the enum definition is updated but the code using
it is not.
Prefer constructor-based dependency injection over field or setter injection.
The project aims for frequent releases. We achieve this using semantic-release, where each merged PR can create a new release. This allows users of the application to quickly receive bug fixes without waiting for arbitrary release cycles. This only works if the quality of the code and especially the tests is up-to-par.