diff options
author | OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com> | 2018-10-29 15:29:27 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-10-29 15:29:27 -0700 |
commit | 5e071f47c08465138cda4fd8929294259cf36bb3 (patch) | |
tree | 7b05798abd00e731c8832f0fe39347feb97d2dd4 | |
parent | a1b961af50495e2887472c8ecc8383414441c6de (diff) | |
parent | a36974585f67171bafed13a6a972364371530d1b (diff) | |
download | podman-5e071f47c08465138cda4fd8929294259cf36bb3.tar.gz podman-5e071f47c08465138cda4fd8929294259cf36bb3.tar.bz2 podman-5e071f47c08465138cda4fd8929294259cf36bb3.zip |
Merge pull request #1720 from vrothberg/contributing-describe-changes
CONTRIBUTING.md: add section about describing changes
-rw-r--r-- | CONTRIBUTING.md | 73 |
1 files changed, 71 insertions, 2 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa95bfe3a..c4e208894 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,11 +52,80 @@ too since in the end the entire PR will be reviewed anyway. When in doubt, squash. PRs that fix issues should include a reference like `Closes #XXXX` in the -commit message so that github will automatically close the referenced issue +commit message so that GitHub will automatically close the referenced issue when the PR is merged. PRs will be approved by an [approver][owners] listed in [`OWNERS`](OWNERS). +### Describe your Changes in Commit Messages + +Describe your problem. Whether your patch is a one-line bug fix or 5000 lines +of a new feature, there must be an underlying problem that motivated you to do +this work. Convince the reviewer that there is a problem worth fixing and that +it makes sense for them to read past the first paragraph. + +Describe user-visible impact. Straight up crashes and lockups are pretty +convincing, but not all bugs are that blatant. Even if the problem was spotted +during code review, describe the impact you think it can have on users. Keep in +mind that the majority of users run packages provided by distributions, so +include anything that could help route your change downstream. + +Quantify optimizations and trade-offs. If you claim improvements in +performance, memory consumption, stack footprint, or binary size, include +numbers that back them up. But also describe non-obvious costs. Optimizations +usually aren’t free but trade-offs between CPU, memory, and readability; or, +when it comes to heuristics, between different workloads. Describe the expected +downsides of your optimization so that the reviewer can weigh costs against +benefits. + +Once the problem is established, describe what you are actually doing about it +in technical detail. It’s important to describe the change in plain English for +the reviewer to verify that the code is behaving as you intend it to. + +Solve only one problem per patch. If your description starts to get long, +that’s a sign that you probably need to split up your patch. + +If the patch fixes a logged bug entry, refer to that bug entry by number and +URL. If the patch follows from a mailing list discussion, give a URL to the +mailing list archive. + +However, try to make your explanation understandable without external +resources. In addition to giving a URL to a mailing list archive or bug, +summarize the relevant points of the discussion that led to the patch as +submitted. + +If you want to refer to a specific commit, don’t just refer to the SHA-1 ID of +the commit. Please also include the oneline summary of the commit, to make it +easier for reviewers to know what it is about. Example: + +``` +Commit f641c2d9384e ("fix bug in rm -fa parallel deletes") [...] +``` + +You should also be sure to use at least the first twelve characters of the +SHA-1 ID. The libpod repository holds a lot of objects, making collisions with +shorter IDs a real possibility. Bear in mind that, even if there is no +collision with your six-character ID now, that condition may change five years +from now. + +If your patch fixes a bug in a specific commit, e.g. you found an issue using +git bisect, please use the ‘Fixes:’ tag with the first 12 characters of the +SHA-1 ID, and the one line summary. For example: + +``` +Fixes: f641c2d9384e ("fix bug in rm -fa parallel deletes") +``` + +The following git config settings can be used to add a pretty format for +outputting the above style in the git log or git show commands: + +``` +[core] + abbrev = 12 +[pretty] + fixes = Fixes: %h (\"%s\") +``` + ### Sign your PRs The sign-off is a line at the end of the explanation for the patch. Your @@ -127,7 +196,7 @@ Integration Tests [README.md](test/README.md). For general questions and discussion, please use the IRC `#podman` channel on `irc.freenode.net`. -For discussions around issues/bugs and features, you can use the github +For discussions around issues/bugs and features, you can use the GitHub [issues](https://github.com/containers/libpod/issues) and [PRs](https://github.com/containers/libpod/pulls) |