From 9c6b7821963e15c68065d5c89a15f757b7ebe070 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 11 Mar 2020 15:57:20 +0100 Subject: [PATCH] Merge #18283: doc: Explain rebase policy in CONTRIBUTING.md fa1244783ccb4798af4906ac6be5a38df241fa38 doc: Explain rebase/squash policy in CONTRIBUTING.md (MarcoFalke) Pull request description: ACKs for top commit: hebasto: re-ACK fa1244783ccb4798af4906ac6be5a38df241fa38, typos fixed. laanwj: ACK fa1244783ccb4798af4906ac6be5a38df241fa38 Tree-SHA512: b8f790a8ffa4f59f26b64befbea8acd92f3c548e577c35750a60dde94b4340258a540ad0db60a6de650db0156cbff9e1330c589c300db5253531fe6a16cdbcc9 --- CONTRIBUTING.md | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2df52f4e5c..7f2b853007 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -103,12 +103,12 @@ 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 @@ -135,6 +135,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 -----------------------