Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instructions to rebase and push branch before merging #267

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion work/flow.rst
Original file line number Diff line number Diff line change
Expand Up @@ -479,11 +479,24 @@ We **always use non-fast forward merges** so that the merge point is marked in G

.. code-block:: bash

# Prepare to merge
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this comment. Instead, before the code block, write:

First, prepare the merge by making sure the branch is up-to-date:

git checkout master
git pull # Sanity check; if master is up-to-date, skip ahead to "Merge" below
git checkout tickets/DM-NNNN
git rebase master
git push --force # so that the commits on the remote branch match the commits merged*
# Merge
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this comment.

Now add the paragraph + list starting with "We force push..." here.

Then introduce a new code block:

Next, merge to ``master`` and push.
We **always use non-fast forward merges** so that the merge point is marked in Git history, with the merge commit containing the ticket number:

.. code-block:: bash

   git checkout master
   git merge --no-ff tickets/DM-NNNN
   git push

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means you can delete the identical paragraph, We **always use ... from before the two code blocks.

git checkout master
git pull # Sanity check; rebase ticket if master was updated.
git merge --no-ff tickets/DM-NNNN
git push

We force push the rebased branch for three reasons:

1. In many repos, GitHub branch protection requires that Travis was run on any commits before they can be pushed onto master.
Branch protections also require that the branch is up-to-date with ``master`` before a merge is allowed.
2. The policy is to delete branches that have been merged. This is only possible if the exact commit has been merged.
3. For convenience, GitHub will automatically close pull requests if the corresponding branch has been merged to master.

**GitHub pull request pages also offer a 'big green button' for merging a branch to master**.
We discourage you from using this button since there isn't a convenient way of knowing that the merged development history graph will be linear from GitHub's interface.
Rebasing the ticket branch against ``master`` and doing the non-fast forward merging on the command line is the safest workflow.
Expand Down