Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Once your changes and tests are ready to submit for review:

1. Submit a pull request

Push your local changes to your forked copy of the repository and [submit a pull request](https://help.github.com/articles/about-pull-requests/). In the pull request, choose a title which sums up the changes that you have made, and in the body provide more details about what your changes do. Also mention the number of the issue where discussion has taken place, eg `Closes #123`.
Push your local changes to your forked copy of the repository and [submit a pull request](https://help.github.com/articles/about-pull-requests/). In the pull request, choose a title which sums up the changes that you have made, and in the body provide more details about what your changes do. Also mention the number of the issue where discussion has taken place, eg `Closes #123`. See more information about pull requests [below](#pull-requests)

Then sit back and wait. There will probably be discussion about the pull request and, if any changes are needed, we would love to work with you to get your pull request merged into ml-cpp.

Expand All @@ -64,3 +64,42 @@ Please adhere to the general guideline that you should never force push to a pub
1. If you change code, follow the existing [coding style](STYLEGUIDE.md).
1. Write a test, unit tests are located under `lib/{module}/unittest`
1. Test your changes (`make test`)

## Pull Requests

Every change made to ml-cpp must be held to a high standard, Pull Requests are equally important as they document changes and decissions that have been made. `You Know, for Search` - a descriptive and relevant summary of the change helps people to find your PR later on.

The followings lists some guidelines when authoring pull requests. Note, try to follow this guideline as close as possible but do not feel bad if you are unsure, team members can help you.

1. PR title summarizes the change, short and descriptive.
1. Use prefixes for quick categorization:
1. [X.Y] Branch label if backport PR, if omited the PR only applies to master. Note, backport PR's are done after master PR, further explanation can be found below [#backport]
1. [ML] mandatory prefix, to be consistent with other repositories
1. [FEATURE] If your pull requests targets a feature branch, this prefix helps as a filter
1. A detailed summary of what changed. Some hints:
1. Keep it short but do not ommit important details. Usually we squash and merge, write what you would write as commit message of the squashed commit, later you can reuse what you wrote.
1. A link to each issue that is closed by the PR (e.g. Closes #123) or related (Relates #456)
1. Further information if necessary, maybe you want to share a screenshot, list open Todo's, describe your thought process, etc.
1. Optional: List of backport PR's, other related PR's
1. Label the PR, not all might apply:
1. `:ml` mandatory label, to be consistent with other repositories.
1. `>type` Type of the PR, e.g. `>bug`, `>refactoring`, `>enhancement`, `>test`.
1. `vX.Y` Versions that PR should be applied to, a PR on master should always contain all backport versions as well, a backport PR should only have one corresponding version.
1. `>non-issue` if the PR is not important for the changelog, e.g. a bugfix for an unreleased feature
1. `affects-results` If the PR is expected to have a significant impact on results(QA test suite), e.g. changes of scoring. Be aware that changes in terms of memory consumption can indirectly lead to significant result changes.
1. `discuss` If your PR suggests a change for which you'd first like to discuss regarding its functional changes before going deep into actual implementation details (e.g. you change a default)
1. `WIP` lets potential reviewers know, that you haven't completed the PR yet and you are still working on further changes, lets reviewers know to maybe wait a bit until you finalized the PR.
1. `review` Try to find reviewers for your change, github has some suggestions, however if unsure or you know the reviewer is very busy, `review` marks the PR as free to grab for review. Still, PR's are open to anyone to comment.

### Backport

Any development usually starts with an implementation on `master`, therefore any PR starts with a PR against `master`. Depending on the type we then decide about backport branches, features usually get backported to the active release branch, e.g. `6.x`, bugfixes might get backported to concrete release branches, e.g. `6.1`. If unsure about the versions to backport to, it can be discussed on the master PR.

A backport starts right after merging the PR on `master`, please open backport PR's immediately after merging to `master`, even if you intend to merge backports a little later (e.g. to wait/analyse QA results). This helps to ensure that backports do not get lost. Rules for Backport PR's:

1. Prefix the title with `[X.Y]` and the mandatory `[ML]`, e.g. `[6.3][ML] Store expanding window bucket values in a compressed format`.
1. Link to the originating PR in the description, you do not need to repeat the description as discussions should exclusively happen on the master PR.
1. Backports do not need a review, however if you had to do further changes (merge conflicts, compiler specialities) it's advised to let the adjustments get quickly reviewed.
1. Label the PR with `>backport` and the corresponding version of this backport, the full set of labels remain on the master PR. If the master PR is classified as `>non-issue` also add the `>non-issue` label due to CI checks.

Although it might look simple, always wait for CI to confirm that changes are sound to avoid unnecessary build breaks.