@@ -62,7 +62,54 @@ Whether you are contributing to civicrm-core or an ancillary project (using GitH
...
@@ -62,7 +62,54 @@ Whether you are contributing to civicrm-core or an ancillary project (using GitH
See also: [reviewing someone else's pull request](/core/pr-review.md)
See also: [reviewing someone else's pull request](/core/pr-review.md)
## Specific steps
## Pull requests {:#pr}
!!! note "terminology"
The terms "pull request", "merge request", "PR", and "MR" all effectively synonymous. GitHub uses "pull request", and GitLab uses "merge request".
### Creating a pull request {:#pr-submit}
1. In the web browser, navigate to the web page for your fork (e.g. `https://github.com/myuser/civicrm-core` )
1. Click **Pull Request**
1. There will be two branches specified – the (first) left should be "civicrm" (i.e. where the code is going to). The second (right) should be your branch.
1. Add a [good subject](#pr-subject) and explanation, and submit.
### Writing a pull request subject {:#pr-subject}
Pull request titles don't need to be identical to issue titles, and in particular, you may want to focus more positively on the changes in code than on the broader feature changes. Here are some guidelines for writing a good subject line:
When filing a pull-request, use a descriptive subject. These are good examples:
*`CRM-12345 - Fix Paypal IPNs when moon is at half-crescent (waxing)`
***CRM-_XXXXX_** - This is a reference to the [Jira issue tracker](/tools/issue-tracking.md#jira). A bot will setup crosslinks between JIRA and GitHub.
***Description** - Provide a brief description of what the pull-request does.
***(WIP)** - "Work in Progress" - If you are still developing a set of
changes, it may be useful to submit a pull-request and flag it as
`(WIP)`. This allows you to have discussion with other developers and
check test results. Once the change is ready, update the subject line
to remove `(WIP)`.
***(NFC)** - "Non-Functional Change" - Most patches are designed to
change functionality (e.g. fix an error message or add a new button).
However, some changes are non-functional -- e.g. they cleanup the
code-style, improve the comments, or improve the test-suite.
### Pull-Request Scope {:#pr-scope}
A good pull request addresses a clearly-defined problem. There should be a detailed description logged in the [issue tracker](http://issues.civicrm.org/). Excellent PRs also increase test coverage. If you are tempted to do additional tweaks or code cleanup outside the scope of that issue, you could make a separate commit and include them in the PR if they are minor & non-controversial, or create a seperate PR if they are more complex.
There is no size limit for PRs as long as they are focused on completely solving a discreet problem. As a practical matter, though, bigger PRs may take longer to review and merge. When possible, split "epic" issues into bite-sized chunks as long as each seperate PR is functionally complete and does not cause merge conflicts with your other PRs. In the latter case, add commits to an existing PR.
### Reviewing a pull request
See [How to review a core pull request](/core/pr-review.md)
## Git tasks
### Cloning {:#cloning}
### Cloning {:#cloning}
...
@@ -164,53 +211,6 @@ Follow these guidelines to write a good commit messages:
...
@@ -164,53 +211,6 @@ Follow these guidelines to write a good commit messages:
* be wrapped at 72 characters
* be wrapped at 72 characters
* explain *what*, *why*, and *how*
* explain *what*, *why*, and *how*
### Pull requests {:#pr}
!!! note "terminology"
The terms "pull request", "merge request", "PR", and "MR" all effectively synonymous. GitHub uses "pull request", and GitLab uses "merge request".
#### Creating a pull request {:#pr-submit}
1. In the web browser, navigate to the web page for your fork (e.g. `https://github.com/myuser/civicrm-core` )
1. Click **Pull Request**
1. There will be two branches specified – the (first) left should be "civicrm" (i.e. where the code is going to). The second (right) should be your branch.
1. Add a [good subject](#pr-subject) and explanation, and submit.
#### Writing a pull request subject {:#pr-subject}
Pull request titles don't need to be identical to issue titles, and in particular, you may want to focus more positively on the changes in code than on the broader feature changes. Here are some guidelines for writing a good subject line:
When filing a pull-request, use a descriptive subject. These are good examples:
*`CRM-12345 - Fix Paypal IPNs when moon is at half-crescent (waxing)`
***CRM-_XXXXX_** - This is a reference to the [Jira issue tracker](/tools/issue-tracking.md#jira). A bot will setup crosslinks between JIRA and GitHub.
***Description** - Provide a brief description of what the pull-request does.
***(WIP)** - "Work in Progress" - If you are still developing a set of
changes, it may be useful to submit a pull-request and flag it as
`(WIP)`. This allows you to have discussion with other developers and
check test results. Once the change is ready, update the subject line
to remove `(WIP)`.
***(NFC)** - "Non-Functional Change" - Most patches are designed to
change functionality (e.g. fix an error message or add a new button).
However, some changes are non-functional -- e.g. they cleanup the
code-style, improve the comments, or improve the test-suite.
#### Pull-Request Scope {:#pr-scope}
A good pull request addresses a clearly-defined problem. There should be a detailed description logged in the [issue tracker](http://issues.civicrm.org/). Excellent PRs also increase test coverage. If you are tempted to do additional tweaks or code cleanup outside the scope of that issue, you could make a separate commit and include them in the PR if they are minor & non-controversial, or create a seperate PR if they are more complex.
There is no size limit for PRs as long as they are focused on completely solving a discreet problem. As a practical matter, though, bigger PRs may take longer to review and merge. When possible, split "epic" issues into bite-sized chunks as long as each seperate PR is functionally complete and does not cause merge conflicts with your other PRs. In the latter case, add commits to an existing PR.
#### Reviewing a pull request
See [How to review a core pull request](/core/pr-review.md)
### Rebasing {:#rebasing}
### Rebasing {:#rebasing}
Sometimes when you [make a pull request](#pr) someone else merges a change into the upstream repository that *conflicts* with your change. The best way to resolve this conflict is to rebase. It's a good idea to [read about rebasing](https://git-scm.com/docs/git-rebase) so you understand the theory. Here's the practice:
Sometimes when you [make a pull request](#pr) someone else merges a change into the upstream repository that *conflicts* with your change. The best way to resolve this conflict is to rebase. It's a good idea to [read about rebasing](https://git-scm.com/docs/git-rebase) so you understand the theory. Here's the practice:
