-
Notifications
You must be signed in to change notification settings - Fork 54
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #312 from ceph/mergify/bp/quincy/pr-310
Documentation: add CONTRIBUTING.MD (backport #310)
- Loading branch information
Showing
1 changed file
with
75 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
# Contributing to cephadm-ansible | ||
|
||
1. Follow the [commit guidelines](#commit-guidelines) | ||
|
||
## Commit guidelines | ||
|
||
- All commits should have a subject and a body | ||
- The commit subject should briefly describe what the commit changes | ||
- The commit body should describe the problem addressed and the chosen solution | ||
- What was the problem and solution? Why that solution? Were there alternative ideas? | ||
- Wrap commit subjects and bodies to 80 characters | ||
- Sign-off your commits | ||
- Add a best-effort scope designation to commit subjects. This could be a directory name, file name, | ||
or the name of a logical grouping of code. Examples: | ||
- library: add a placeholder module for the validate action plugin | ||
- site.yml: combine validate play with fact gathering play | ||
- Commits linked with an issue should trace them with : | ||
- Fixes: #2653 | ||
|
||
[Suggested reading.](https://chris.beams.io/posts/git-commit/) | ||
|
||
## Pull requests | ||
|
||
### Jenkins CI | ||
|
||
We use Jenkins to run several tests on each pull request. | ||
|
||
If you don't want to run a build for a particular pull request, because all you are changing is the | ||
README for example, add the text `[skip ci]` to the PR title. | ||
|
||
### Merging strategy | ||
|
||
Merging PR is controlled by [mergify](https://mergify.io/) by the following rules: | ||
|
||
- at least one approuval from a maintainer | ||
- a SUCCESS from the CI pipeline "cephadm-ansible PR Pipeline" | ||
|
||
If you work is not ready for review/merge, please request the DNM label via a comment or the title of your PR. | ||
This will prevent the engine merging your pull request. | ||
|
||
### Backports (maintainers only) | ||
|
||
If you wish to see your work from 'main' being backported to a stable branch you can ping a maintainer | ||
so he will set the backport label on your PR. Once the PR from main is merged, a backport PR will be created by mergify, | ||
if there is a cherry-pick conflict you must resolv it by pulling the branch. | ||
|
||
**NEVER** push directly into a stable branch, **unless** the code from main has diverged so much that the files don't exist in the stable branch. | ||
If that happens, inform the maintainers of the reasons why you pushed directly into a stable branch, if the reason is invalid, maintainers will immediatly close your pull request. | ||
|
||
### Keep your branch up-to-date | ||
|
||
Sometimes, a pull request can be subject to long discussion, reviews and comments, meantime, `main` | ||
moves forward so let's try to keep your branch rebased on main regularly to avoid huge conflict merge. | ||
A rebased branch is more likely to be merged easily & shorter. | ||
|
||
### Organize your commits | ||
|
||
Do not split your commits unecessary, we are used to see pull request with useless additional commits like | ||
"I'm addressing reviewer's comments". So, please, squash and/or amend them as much as possible. | ||
|
||
Similarly, split them when needed, if you are modifying several parts in cephadm-ansible or pushing a large | ||
patch you may have to split yours commit properly so it's better to understand your work. | ||
Some recommandations: | ||
|
||
- one fix = one commit, | ||
- do not mix multiple topics in a single commit, | ||
- if you PR contains a large number of commits that are each other totally unrelated, it should probably even be split in several PRs. | ||
|
||
If you've broken your work up into a set of sequential changes and each commit pass the tests on their own then that's fine. | ||
If you've got commits fixing typos or other problems introduced by previous commits in the same PR, then those should be squashed before merging. | ||
|
||
If you are new to Git, these links might help: | ||
|
||
- [https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History) | ||
- [http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html](http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html) |