From b470bb214c141551a85d9ef30b798d356bbc782a Mon Sep 17 00:00:00 2001 From: Doug Ilijev Date: Mon, 8 Oct 2018 16:35:14 -0700 Subject: [PATCH 1/2] Add a guide for rebasing and squashing PRs. --- Rebasing-PRs.md | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ _Sidebar.md | 1 + 2 files changed, 86 insertions(+) create mode 100644 Rebasing-PRs.md diff --git a/Rebasing-PRs.md b/Rebasing-PRs.md new file mode 100644 index 0000000..fc4d5d6 --- /dev/null +++ b/Rebasing-PRs.md @@ -0,0 +1,85 @@ +We ask that PRs, in most cases, should be a single commit based on a reasonably recent commit +(though not necessarily the latest) from the target branch, with no merge commits. + + +# Getting starting and creating a PR + +Following the usual Git convention, the following examples assume the remote referring to your fork [username/ChakraCore](https://github.com/username/ChakraCore), which you cloned from, is named `origin`, and [Microsoft/ChakraCore](https://github.com/Microsoft/ChakraCore) is named `upstream`. + +Assume that your PR branch is named `feature` which you've pushed to your fork (`origin`), such that your pull request branch (locally) is `origin/feature`. + +For various reasons, including simplifying instructions, we recommend not working directly in the `master` branch. Instead, check out a new branch before you start working: + +```sh +(git:master)$ git checkout -b feature # check out a new branch called `feature` +(git:feature)$ # make some changes +(git:feature *)$ git commit -m "Short description of change." +(git:feature)$ git push origin feature # push your changes to your fork (origin) so that you can open a PR. +``` + + +# ChakraCore Jenkins CI and auto-merging + +Due to the speed that changes come in versus the time it takes for Jenkins CI checks to complete, +we do not enforce that the commit is based on the latest commit. +Therefore, please feel free to ignore the following message, which comes from GitHub itself, +rather than from ChakraCore policy or Jenkins. + +> This branch is out-of-date with the base branch. +> +> Merge the latest changes from `master` into this branch. + +The above message is simply saying that `master` has moved ahead of where it was when you started +working on your feature branch. There are no conflicts. +In this case, clicking the button to merge `master` into your branch will cause Jenkins checks +to start over and will delay your pull request from being merged. + +Generally, there is no value in updating your PR in this case, unless you expect that the changes +added to the target branch (e.g. `master`) might cause your change to break, +even though there are no code conflicts. + +However, if you get the following message: + +> This branch has conflicts that must be resolved +> +> Use the web editor or the command line to resolve conflicts. + +In this case you must resolve the conflicts and update your PR. +When you update your PR, we prefer that you rebase or squash your PR (see below) +to ensure that there are no merge commits. + + +# Rebasing your PR + +Rather than merging the target branch into your PR, +we ask that you rebase your PR on top of the latest version of the target branch. +If your target branch is `master`, the simplest way will look something like the following: + +```sh +(git:master)$ git checkout feature +(git:feature)$ git fetch upstream # get the latest commits from upstream (including upstream/master) +(git:feature)$ git rebase upstream/master # follow the prompts to complete the rebase +(git:feature)$ git push --force origin feature # force-push the PR branch to update +``` + + +# Squashing your PR + +While you are still iterating on your PR and getting code reviews, feel free to push additional commits to your PR; +however, before merging your PR, we will generally ask that you squash your PR to a single commit, to simplify history. + +Squashing is similar to rebasing except that the result is exactly one commit. +This is the simplest way to reduce many commits into a single commit. +It is especially helpful in untangling complex history caused by multiple merges into your feature branch +Think of a squash commit as taking your entire PR branch and making a single commit out of the sum of those changes. + +```sh +(git:feature)$ git branch -m feature-saved # rename the current branch so that your work is saved +(git:feature-saved)$ git fetch upstream +(git:feature-saved)$ git checkout -B master upstream/master # forcibly check out a new copy of upstream/master +(git:master)$ git checkout -b feature # create a new branch named 'feature' to match your PR branch +(git:feature)$ git merge --squash feature-saved +(git:feature *)$ git commit # Remove "squashed commit of the following" and subsequent lines. +(git:feature)$ git push --force origin feature # force-push your PR branch to update your PR +``` + diff --git a/_Sidebar.md b/_Sidebar.md index 7d5664c..6effa84 100644 --- a/_Sidebar.md +++ b/_Sidebar.md @@ -21,6 +21,7 @@ * [[Jenkins Build Triggers]] * [[Jenkins Repro Steps]] * [[External Issues]] + * [[Rebasing PRs]] * Engineering Notes * [[String Internals]] * [[Embedding ChakraCore]] From 845d489d4d8273d6395dc02310afed76733b8248 Mon Sep 17 00:00:00 2001 From: Doug Ilijev Date: Tue, 9 Oct 2018 14:20:15 -0700 Subject: [PATCH 2/2] Fix typo --- Rebasing-PRs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rebasing-PRs.md b/Rebasing-PRs.md index fc4d5d6..fcfe4f2 100644 --- a/Rebasing-PRs.md +++ b/Rebasing-PRs.md @@ -2,7 +2,7 @@ We ask that PRs, in most cases, should be a single commit based on a reasonably (though not necessarily the latest) from the target branch, with no merge commits. -# Getting starting and creating a PR +# Getting started and creating a PR Following the usual Git convention, the following examples assume the remote referring to your fork [username/ChakraCore](https://github.com/username/ChakraCore), which you cloned from, is named `origin`, and [Microsoft/ChakraCore](https://github.com/Microsoft/ChakraCore) is named `upstream`.