Merge remote-tracking branch 'upstream/pull/3771'
This commit is contained in:
commit
d2ca26f0e6
1 changed files with 38 additions and 13 deletions
|
@ -23,7 +23,7 @@ so with any new functionality which is written. Tests are also useful
|
||||||
in giving others confidence in the code you've written, and can
|
in giving others confidence in the code you've written, and can
|
||||||
greatly speed up the process of merging in new code.
|
greatly speed up the process of merging in new code.
|
||||||
|
|
||||||
When hacking, you should:
|
When contributing, you should:
|
||||||
|
|
||||||
* Write new tests to cover the new functionality you've added.
|
* Write new tests to cover the new functionality you've added.
|
||||||
* Where appropriate, modify existing tests to reflect new or changed
|
* Where appropriate, modify existing tests to reflect new or changed
|
||||||
|
@ -56,9 +56,9 @@ more importantly, **why** it does that. Good comments help your fellow
|
||||||
developers to read the code and satisfy themselves that it's doing the
|
developers to read the code and satisfy themselves that it's doing the
|
||||||
right thing.
|
right thing.
|
||||||
|
|
||||||
When hacking, you should:
|
When contributing, you should:
|
||||||
|
|
||||||
* Comment your code - don't go overboard, but explain the bits which
|
* Comment your code where necessary - explain the bits which
|
||||||
might be difficult to understand what the code does, why it does it
|
might be difficult to understand what the code does, why it does it
|
||||||
and why it should be the way it is.
|
and why it should be the way it is.
|
||||||
* Check existing comments to ensure that they are not misleading.
|
* Check existing comments to ensure that they are not misleading.
|
||||||
|
@ -81,14 +81,14 @@ to the descriptive texts are welcome.
|
||||||
|
|
||||||
## Committing
|
## Committing
|
||||||
|
|
||||||
When you submit patches, the project maintainer has to read them and
|
When you submit your changes, the project maintainers have to read them and
|
||||||
understand them. This is difficult enough at the best of times, and
|
understand them. This is difficult enough at the best of times, and
|
||||||
misunderstanding patches can lead to them being more difficult to
|
misunderstanding commits can lead to them being more difficult to
|
||||||
merge. To help with this, when submitting you should:
|
merge. To help with this, when committing you should:
|
||||||
|
|
||||||
* Split up large patches into smaller units of functionality.
|
* Split up large commits into smaller units of functionality.
|
||||||
* Keep your commit messages relevant to the changes in each individual
|
* Keep your commit messages relevant to the changes in each individual
|
||||||
unit.
|
commit.
|
||||||
|
|
||||||
When writing commit messages please try and stick to the same style as
|
When writing commit messages please try and stick to the same style as
|
||||||
other commits, namely:
|
other commits, namely:
|
||||||
|
@ -100,12 +100,37 @@ other commits, namely:
|
||||||
For simple commits the one line summary is often enough and the body
|
For simple commits the one line summary is often enough and the body
|
||||||
of the commit message can be left out.
|
of the commit message can be left out.
|
||||||
|
|
||||||
## Sending the patches
|
## Pull Requests
|
||||||
|
|
||||||
If you have forked on GitHub then the best way to submit your patches is to
|
If you have forked on GitHub then the best way to submit your patches is to
|
||||||
push your changes back to GitHub and then send a "pull request" on GitHub.
|
push your changes back to GitHub and then send a "pull request" on GitHub.
|
||||||
|
|
||||||
Otherwise you should either push your changes to a publicly visible git repository
|
If your pull request is small, for example one or two commits each containing
|
||||||
and send the details to the [rails-dev](https://lists.openstreetmap.org/listinfo/rails-dev)
|
only a few lines of code, then it is easy for the maintainers to review.
|
||||||
list or generate patches with `git format-patch` and send them to the
|
|
||||||
[rails-dev](https://lists.openstreetmap.org/listinfo/rails-dev) list.
|
If you are creating a larger pull request, then please help the maintainers
|
||||||
|
with making the reviews as straightforward as possible:
|
||||||
|
|
||||||
|
* The smaller the PR, the easier it is to review. In particular if a PR is too
|
||||||
|
large to review in one sitting, or if changes are requested, then the
|
||||||
|
maintainer needs to repeatedly re-read code that has already been considered.
|
||||||
|
* The commit history is important. This is a large codebase, developed over many
|
||||||
|
years by many developers. We frequently need to read the commit history (e.g.
|
||||||
|
using `git blame`) to figure out what is going on. So small, understandable,
|
||||||
|
and relevant commits are important for other developers looking back at your
|
||||||
|
work in future.
|
||||||
|
|
||||||
|
If you are creating a large pull request then please:
|
||||||
|
|
||||||
|
* Consider splitting your pull request into multiple PRs. If part of your work
|
||||||
|
can be considered standalone, or is a foundation for the rest of your work,
|
||||||
|
please submit it separately first.
|
||||||
|
* Avoid including "fixup" commits. If you have added a fixup commit (for example
|
||||||
|
to fix a rubocop warning, or because you changed your own new code) please
|
||||||
|
combine the fixup commit into the commit that introduced the problem.
|
||||||
|
`git rebase -i` is very useful for this.
|
||||||
|
* Avoid including "merge" commits. If your PR can no longer be merged cleanly
|
||||||
|
(for example, an unrelated change to Gemfile.lock on master now conflicts with
|
||||||
|
your PR) then please rebase your PR onto the latest master. This allows you to
|
||||||
|
fix the conflicts, while keeping the PR a straightforward list of commits. If
|
||||||
|
there are no conflicts, then there is no need to rebase anything.
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue