diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..b82056fe --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,44 @@ +# Contributing + +## License + +By contributing, you agree that your contributions will be licensed under the [GNU General Public License Version 3.0 (GPL-3.0 license)](LICENSE). + +## Contribution process + +This is the process we suggest for contributions. This process is designed to reduce the burden on project reviewers, +impact on other contributors, and to keep the amount of rework from the contributor to a minimum. + +1. Start a discussion by creating a GitHub issue, or asking on Slack (unless the change is trivial, for example a spelling fix in the documentation). + + 1. This step helps you identify possible collaborators and reviewers. + 1. Does the change align with technical vision and project values? + 1. Will the change conflict with another change in progress? If so, work with others to minimize impact. + 1. Is this change large? If so, work with others to break into smaller steps. + +1. Implement the change + + 1. Create or update your own fork of the repository. + 1. If the change is large, post a draft GitHub pull request. + 1. Include tests and documentation as necessary. + 1. Follow the commit message guidelines and other suggestions from the [development guidelines](DEVELOPMENT.md). + +1. Create a GitHub pull request (PR). + + 1. If you already have a draft PR, change it to ready for review. + 1. Refer to the GitHub documentation for more details about collaborating with PRs. + 1. Make sure the pull request passes the tests in CI. + 1. Code reviewers are automatically assigned. + +1. Review is performed by one or more reviewers. + + 1. This normally happens within a few days, but may take longer if the change is large, complex, or if a critical reviewer is unavailable. (feel free to ping the reviewer or team on the pull request). + +1. Address concerns and update the pull request. + + 1. Comments are addressed to each individual commit in the pull request, and changes should be addressed in a new fixup! commit placed after each commit. This is to make it easier for the reviewer to see what was updated. + 1. After pushing the changes, add a comment to the pull-request, mentioning the reviewers by name, stating that the review comments have been addressed. This is the only way that a reviewer is notified that you are ready for the code to be reviewed again. + +1. Maintainer merges the pull request after final changes are accepted. + +1. Merging your improvements will usually trigger a new release once merged diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md new file mode 100644 index 00000000..894c2c7c --- /dev/null +++ b/DEVELOPMENT.md @@ -0,0 +1,123 @@ +# Development + +Inspired by . + +## Commits and pull requests + +### Format Git commit messages + +We format commit messages to adhere to the [conventional commit standard](https://www.conventionalcommits.org/en/v1.0.0/). +The commit messages are also used to automatically create and version releases using [semantic-release](https://semantic-release.gitbook.io/semantic-release). + +### Git merge strategy + +Pull requests are usually merged into `master` using the [`rebase and merge`](https://docs.github.com/en/github/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#rebase-and-merge-your-pull-request-commits) 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. + +## Code Style + +We recommend you use IntelliJ as your IDE. The code style used is the [Google Java style](https://google.github.io/styleguide/javaguide.html). +It is automatically enforced using [spotless](https://github.com/diffplug/spotless). + +To run spotless and other checks before opening a PR: `./gradlew :check` + +In addition to those you should also adhere to the following: + +### Readability + +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. + +### Consistency + +Keep code consistent with surrounding code where possible. + +### Alphabetize + +Alphabetize sections in the documentation source files (both in the table of +contents files and other regular documentation files). + +### Use streams + +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. + +### Prefer String formatting + +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 ternary operator + +Avoid using the ternary operator except for trivial expressions. + +### Define class API for private inner classes too + +It is suggested to declare members in private inner classes as public if they +are part of the class API. + +### Use AssertJ + +Prefer AssertJ for complex assertions. + +### Maintain production quality for test code + +Maintain the same quality for production and test code. + +### Avoid abbreviations + +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 default clause in exhaustive enum-based switch statements + +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. + +### Use constructor injection + +Prefer [constructor-based dependency injection](https://docs.spring.io/spring-framework/reference/core/beans/dependencies/factory-collaborators.html#beans-constructor-injection) +over field or setter injection. + +## Releases + +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.