diff options
Diffstat (limited to 'CONTRIBUTING.md')
| -rw-r--r-- | CONTRIBUTING.md | 71 |
1 files changed, 55 insertions, 16 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 33c797d799..7216db0500 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,15 +46,15 @@ facilitates social contribution, easy testing and peer review. To contribute a patch, the workflow is as follows: - 1. Fork repository ([only the first time](https://help.github.com/en/articles/fork-a-repo)). + 1. Fork repository ([only for the first time](https://help.github.com/en/articles/fork-a-repo)) 1. Create topic branch 1. Commit patches The project coding conventions in the [developer notes](doc/developer-notes.md) -must be adhered to. +must be followed. -In general [commits should be atomic](https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention) -and diffs should be easy to read. For this reason do not mix any formatting +In general, [commits should be atomic](https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention) +and diffs should be easy to read. For this reason, do not mix any formatting fixes or code moves with actual code changes. Commit messages should be verbose by default consisting of a short subject line @@ -100,7 +100,7 @@ Examples: qt: Add feed bump button log: Fix typo in log message -Note that translations should not be submitted as pull requests, please see +Note that translations should not be submitted as pull requests. Please see [Translation Process](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md) for more information on helping with translations. @@ -113,16 +113,16 @@ patch does together with any justification/reasoning. You should include references to any discussions (for example other tickets or mailing list discussions). -At this stage one should expect comments and review from other contributors. You +At this stage, one should expect comments and review from other contributors. You can add more commits to your pull request by committing them locally and pushing to your fork until you have satisfied all feedback. -Note: Code review is a burdensome but important part of the development process, and as such, certain types of pull requests are rejected. In general, if the **improvements** do not warrant the **review effort** required, the PR has a high chance of being rejected. It is up to the PR author to convince the reviewers that the changes warrant the review effort, and if reviewers are "Concept NAK'ing" the PR, the author may need to present arguments and/or do research backing their suggested changes. +Note: Code review is a burdensome but important part of the development process, and as such, certain types of pull requests are rejected. In general, if the **improvements** do not warrant the **review effort** required, the PR has a high chance of being rejected. It is up to the PR author to convince the reviewers that the changes warrant the review effort, and if reviewers are "Concept NACK'ing" the PR, the author may need to present arguments and/or do research backing their suggested changes. -Squashing Commits ---------------------------- -If your pull request is accepted for merging, you may be asked by a maintainer -to squash and or [rebase](https://git-scm.com/docs/git-rebase) your commits +### Squashing Commits + +If your pull request contains fixup commits (commits that change the same line of code repeatedly) or too fine-grained +commits, you may be asked to [squash](https://git-scm.com/docs/git-rebase#_interactive_mode) your commits before it will be merged. The basic squashing workflow is shown below. git checkout your_branch_name @@ -133,8 +133,8 @@ before it will be merged. The basic squashing workflow is shown below. # Save and quit. git push -f # (force push to GitHub) -Please update the resulting commit message if needed, it should read as a -coherent message. In most cases this means that you should not just list the +Please update the resulting commit message if needed. It should read as a +coherent message. In most cases, this means that you should not just list the interim commits. If you have problems with squashing (or other workflows with `git`), you can @@ -149,6 +149,20 @@ the respective change set. The length of time required for peer review is unpredictable and will vary from pull request to pull request. +### Rebasing Changes + +When a pull request conflicts with the target branch, you may be asked to rebase it on top of the current target branch. +The `git rebase` command will take care of rebuilding your commits on top of the new base. + +This project aims to have a clean git history, where code changes are only made in non-merge commits. This simplifies +auditability because merge commits can be assumed to not contain arbitrary code changes. Merge commits should be signed, +and the resulting git tree hash must be deterministic and reproducible. The script in +[/contrib/verify-commits](/contrib/verify-commits) checks that. + +After a rebase, reviewers are encouraged to sign off on the force push. This should be relatively straightforward with +the `git range-diff` tool explained in the [productivity +notes](/doc/productivity.md#diff-the-diffs-with-git-range-diff). To avoid needless review churn, maintainers will +generally merge pull requests that received the most review attention first. Pull Request Philosophy ----------------------- @@ -173,9 +187,9 @@ in the future, they may be removed by the Repository Maintainer. Refactoring is a necessary part of any software project's evolution. The following guidelines cover refactoring pull requests for the project. -There are three categories of refactoring, code only moves, code style fixes, -code refactoring. In general refactoring pull requests should not mix these -three kinds of activity in order to make refactoring pull requests easy to +There are three categories of refactoring: code-only moves, code style fixes, and +code refactoring. In general, refactoring pull requests should not mix these +three kinds of activities in order to make refactoring pull requests easy to review and uncontroversial. In all cases, refactoring PRs must not change the behaviour of code within the pull request (bugs must be preserved as is). @@ -309,6 +323,31 @@ about: when someone else is asking for feedback on their code, and universe balances out. +Backporting +----------- + +Security and bug fixes can be backported from `master` to release +branches. +If the backport is non-trivial, it may be appropriate to open an +additional PR, to backport the change, only after the original PR +has been merged. +Otherwise, backports will be done in batches and +the maintainers will use the proper `Needs backport (...)` labels +when needed (the original author does not need to worry). + +A backport should contain the following metadata in the commit body: + +``` +Github-Pull: #<PR number> +Rebased-From: <commit hash of the original commit> +``` + +Have a look at [an example backport PR]( +https://github.com/bitcoin/bitcoin/pull/16189). + +Also see the [backport.py script]( +https://github.com/bitcoin-core/bitcoin-maintainer-tools#backport). + Release Policy -------------- |