mirror of
https://github.com/moby/moby.git
synced 2022-11-09 12:21:53 -05:00
CONTRIBUTING: add tips for succesful changes
Expand the contributing doc to include tips for successful PRs and guidance on commit messages. This should help contributers to have better position PRs that are more likely to be merged. Signed-off-by: Stephen J Day <stephen.day@docker.com>
This commit is contained in:
parent
518945b6bd
commit
59e1d579e0
1 changed files with 54 additions and 0 deletions
|
@ -173,10 +173,64 @@ committing your changes. Most editors have plug-ins that do this automatically.
|
|||
Pull request descriptions should be as clear as possible and include a reference
|
||||
to all the issues that they address.
|
||||
|
||||
### Successful Changes
|
||||
|
||||
Before contributing large or high impact changes, make the effort to coordinate
|
||||
with the maintainers of the project before submitting a pull request. This
|
||||
prevents you from doing extra work that may or may not be merged.
|
||||
|
||||
Large PRs that are just submitted without any prior communication are unlikely
|
||||
to be successful.
|
||||
|
||||
While pull requests are the methodology for submitting changes to code, changes
|
||||
are much more likely to be accepted if they are accompanied by additional
|
||||
engineering work. While we don't define this explicitly, most of these goals
|
||||
are accomplished through communication of the design goals and subsequent
|
||||
solutions. Often times, it helps to first state the problem before presenting
|
||||
solutions.
|
||||
|
||||
Typically, the best methods of accomplishing this are to submit an issue,
|
||||
stating the problem. This issue can include a problem statement and a
|
||||
checklist with requirements. If solutions are proposed, alternatives should be
|
||||
listed and eliminated. Even if the criteria for elimination of a solution is
|
||||
frivolous, say so.
|
||||
|
||||
Larger changes typically work best with design documents. These are focused on
|
||||
providing context to the design at the time the feature was conceived and can
|
||||
inform future documentation contributions.
|
||||
|
||||
### Commit Messages
|
||||
|
||||
Commit messages must start with a capitalized and short summary (max. 50 chars)
|
||||
written in the imperative, followed by an optional, more detailed explanatory
|
||||
text which is separated from the summary by an empty line.
|
||||
|
||||
Commit messages should follow best practices, including explaining the context
|
||||
of the problem and how it was solved, including in caveats or follow up changes
|
||||
required. They should tell the story of the change and provide readers
|
||||
understanding of what led to it.
|
||||
|
||||
If you're lost about what this even means, please see [How to Write a Git
|
||||
Commit Message](http://chris.beams.io/posts/git-commit/) for a start.
|
||||
|
||||
In practice, the best approach to maintaining a nice commit message is to
|
||||
leverage a `git add -p` and `git commit --amend` to formulate a solid
|
||||
changeset. This allows one to piece together a change, as information becomes
|
||||
available.
|
||||
|
||||
If you squash a series of commits, don't just submit that. Re-write the commit
|
||||
message, as if the series of commits was a single stroke of brilliance.
|
||||
|
||||
That said, there is no requirement to have a single commit for a PR, as long as
|
||||
each commit tells the story. For example, if there is a feature that requires a
|
||||
package, it might make sense to have the package in a separate commit then have
|
||||
a subsequent commit that uses it.
|
||||
|
||||
Remember, you're telling part of the story with the commit message. Don't make
|
||||
your chapter weird.
|
||||
|
||||
### Review
|
||||
|
||||
Code review comments may be added to your pull request. Discuss, then make the
|
||||
suggested modifications and push additional commits to your feature branch. Post
|
||||
a comment after pushing. New commits show up in the pull request automatically,
|
||||
|
|
Loading…
Reference in a new issue