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.

Sign up for GitHub

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

Updating the merge procedure to removing synchronisation with Perforce repository #228

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Updating the merge procedure to removing synchronisation with Perforce repository #228

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
35949db
Removing synchronisation with Perforce repository.
rptb1 May 30, 2023
9dddbc5
Resolving FIXME comments with links to procedures and issues.
rptb1 May 30, 2023
72b1f65
Correcting cross-references to steps and reintroducing rebase instruc…
rptb1 Jun 12, 2023
38e2a57
Correcting references to "perforce" remote when resolving conflict wi…
rptb1 Nov 6, 2023
d957641
Correcting invocation of `git pull --rebase` to avoid discarding the …
rptb1 Nov 6, 2023
dfec8bc
Generalising instructions for obtaining CI results.
rptb1 Nov 6, 2023
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
156 changes: 41 additions & 115 deletions procedure/pull-request-merge.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ The purpose of this procedure is:

2. protect the MPS from introduction of defects

3. maintain and protect the permanent record of the MPS in Perforce
and Git, so that defects can be discovered, fixed, and prevented
3. maintain and protect the permanent record of the MPS in Git, so
that defects can be discovered, fixed, and prevented

As with any procedure, you can vary this one to meet this purpose, but
you should probably read section "`6. Rationale`_".
Expand Down Expand Up @@ -101,11 +101,14 @@ When you finish the checklist, decide whether to start

#. Is the merge approved by an approving review?

The `Ravenbrook MPS repo on GitHub`_ is set to require an approving
review and GitHub will block a push (step 8) without one. If you
push an unapproved merge to Perforce, this will cause gitpushbot to
fail, and leave Perforce and GitHub out of sync. *Do not push to
Perforce without GitHub approval.*
[Technically, the work to be merged must have passed
`proc.review.exit
<https://github.com/Ravenbrook/mps/blob/branch/2023-01-19/review-procedure/procedure/review.rst#58-review-exit>`_
but at the time of writing, proc.review has not itself been
merged. It is pending in `GitHub pull request #123
<https://github.com/Ravenbrook/mps/pull/123>`_. In practice, we
are applying that procedure. Once we merge the review procedure
itself, this comment should be updated. RB 2023-05-30]

#. Has the contributor licensed their work?

Expand Down Expand Up @@ -163,10 +166,6 @@ When you finish the checklist, decide whether to start

.. _pull request merge branch: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request

.. _Travis CI build history for the repo: https://app.travis-ci.com/github/Ravenbrook/mps/builds

.. _GitHub workflows for the repo: https://github.com/Ravenbrook/mps/actions

.. _MPS interface: https://www.ravenbrook.com/project/mps/master/manual/html/topic/interface.html


Expand All @@ -181,47 +180,13 @@ These steps will only rarely need repeating.
#. If the merge has conflicts, you will need competence in using Git
to resolve merge conflicts.

#. If you want to vary the procedure, you will need to understand how
Perforce Git Fusion [Perforce_2017]_ connects Ravenbrook's Perforce
repository to the `Ravenbrook MPS repo on GitHub`_.

#. Ensure your public SSH key is submitted in Perforce at
//.git-fusion/users/USER/keys/

This is the public key that the SSH used by your Git will use to
connect to perforce.ravenbrook.com.

**NOTE**: As of 2022-11, you may not be able to connect to Perforce
Git Fusion without specifying an old public key algorithm. See
`"Connecting to Git Fusion"
<https://info.ravenbrook.com/mail/2022/11/29/21-01-14/0/>`__.

#. Ensure your e-mail address is submitted in Perforce at
//.git-fusion/users/p4gf_usermap and matches your Perforce user
record.

#. Clone the Ravenbrook MPS GitHub repository and name the remote
"github" [#github]_. This will give you access to CI to
build and test the merge. (If you're an MPS developer you can use
your existing repo.) ::

git clone -o github [email protected]:Ravenbrook/mps.git

#. Set your e-mail address for commits to the repo to match the one in
your Perforce user record, e.g. from within the "mps" repo
directory ::

git config user.email [email protected]

and possibly your name if you don't have that set in Git globally ::

git config user.name 'Julius Cesar'

#. Add the Git Fusion mps-public repo, which is the interface to
Ravenbrook's Perforce. From within the "mps" repo directory ::

git remote add perforce ssh://[email protected]:1622/mps-public

.. [#github] There's nothing special about this name -- it's just
assumed in the examples in the procedure.

Expand All @@ -231,7 +196,7 @@ These steps will only rarely need repeating.
5. Merging procedure
--------------------

Note: At any point before a successful "push" in step 7, this
Note: At any point before a successful "push" in step 6, this
procedure can be abandoned without harm. All work is local to your
working repo before that point.

Expand Down Expand Up @@ -263,35 +228,11 @@ working repo before that point.
git remote add captain-contrib https://gitlab.com/captcontrib/mps.git
git fetch captain-contrib mps-speed-hax:branch/2023-01-06/speed-hax

Double check you've got the branch name right. Using the wrong
branch naming `causes permanent pollution in the Ravenbrook
Perforce repository
<https://info.ravenbrook.com/mail/2023/01/07/15-06-41/0/>`_.

2. Optionally, let other people know that you're working on a merge
into master. Negotiate to avoid racing them to push to the master
codeline (step 7) because that will create extra merging work.

3. Ensure your local master is up to date with Perforce::
codeline (step 6) because that will create extra merging work.

git pull --ff-only perforce master

If you get an error, then GitHub's master and Perforce's master are
in out of sync, and this procedure fails.

Ensure your local master is not ahead of Perforce::

git push --dry-run perforce

If this shows anything other than "Everything up-to-date." then
GitHub's master and Perforce's master are in out of sync, and this
procedure fails.

[It may be possible to fix that here and now and continue. That's
a subject for a whole nother procedure that we don't currently
have. RB 2023-01-12]

4. Merge the branch in to your local master::
3. Merge the branch in to your local master::

git merge --no-ff branch/2023-01-06/speed-hax

Expand All @@ -310,10 +251,10 @@ working repo before that point.
branch. If you still can't resolve conflicts, this procedure
fails.

5. Maybe build and test locally. If either
4. Maybe build and test locally. If either

- the merge was non-trivial
- there has been any rebasing (see step 7)
- there has been any rebasing (see step 6)
- there were failed or missing build results from CI

then build and test the merge result locally if possible. For
Expand All @@ -325,14 +266,14 @@ working repo before that point.
platforms.

If tests do not pass, review your conflict resolution from the
merge (step 4), and if that doesn't fix things, the procedure
merge (step 3), and if that doesn't fix things, the procedure
fails, and you need to go back to the source of the branch,
e.g. the pull request and its original author. Something's wrong!

6. Maybe build and test using CI. As with step 5, if either
5. Maybe build and test using CI. As with step 4, if either

- the merge was non-trivial
- there has been any rebasing (see step 7)
- there has been any rebasing (see step 6)
- there were failed or missing build results from CI

then push the merge to a fresh branch in the `Ravenbrook MPS repo
Expand All @@ -341,15 +282,14 @@ working repo before that point.

git push github HEAD:merge/2023-01-06/speed-hax

You will need to wait for results from CI. Look for a build
results in the `Travis CI build history for the repo`_ and in the
`GitHub workflows for the repo`_.
You will need to wait for `results from CI
<../design/tests.txt#continuous-integration>`_.

See build (step 5) about what to do if tests do not pass.
See build (step 4) about what to do if tests do not pass.

7. Submit your merged master and the branch to Perforce::
6. Submit your merged master and the branch to GitHub::

git push perforce master branch/2023-01-06/speed-hax
git push github master branch/2023-01-06/speed-hax

**Important**: Do *not* force this push.

Expand All @@ -358,41 +298,23 @@ working repo before that point.

You can attempt to rebase your work on those changes::

git pull --rebase perforce
git pull --rebase=merges github

then go back to testing (step 5).
then go back to testing (step 4).

Alternatively, you could undo your merging work::

git reset --hard perforce/master

then go back to merging (step 4).

8. Optionally, if and *only if* the Perforce push (step 7) succeeded,
you can also push to GitHub::

git push github master branch/2023-01-06/speed-hax

If you don't do this, then within `30 minutes
<https://info.ravenbrook.com/infosys/robots/gitpushbot/etc/crontab>`_
check that the merge appears in `the commits in the Ravenbrook MPS
repo on GitHub <https://github.com/Ravenbrook/mps/commits/master>`_.

If they do not appear:

1. Check email for error messages from gitpushbot and resolve them.
git reset --hard github/master

2. Check (or ask a sysadmin to check) that gitpushbot is running
on Berunda and restart it if necessary, or ask a sysadmin to do
this.
then go back to merging (step 3).

9. Eyeball the pull request and related issues on GitHub to make sure
7. Eyeball the pull request and related issues on GitHub to make sure
the merge was recorded correctly. Check that any issues *not
completely resolved* by the merge were not closed. Re-open them if
necessary.

10. Edit the comment you made in `3. Pre-merge checklist`_ to record
the end time of the merge and how long you spent merging, like::
8. Edit the comment you made in `3. Pre-merge checklist`_ to record
the end time of the merge and how long you spent merging, like::

6. End time 11:20. Merge took 17 mins.

Expand Down Expand Up @@ -451,10 +373,10 @@ and so we should not edit it.
6.2. Why not press the GitHub merge button?
...........................................

We cannot use the GitHub pull request merge button because it would
put the GitHub master branch out of sync with (ahead of) Perforce.
Currently, Perforce is the authoritative home of the MPS, and the Git
repository is a mirror.
In the past, we could not use the GitHub pull request merge button
because it would have put the GitHub master branch out of sync with
(ahead of) Perforce. However, Git is now the authoritative home of
the MPS, and the Perforce repository is mothballed.

According to `GitHub's "About pull request merges"
<https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges>`_:
Expand All @@ -477,12 +399,15 @@ says:
GITHUB_SHA for this event is the last merge commit of the pull
request merge branch.

So, `once Git becomes the home
<https://github.com/Ravenbrook/mps/issues/98>`_ we will be able to use
the button to to replace sections 4 and 5, the procedure, but not
Now that `Git has becomes the home
<https://github.com/Ravenbrook/mps/issues/98>`_ we could potentially
use the button to to replace sections 4 and 5, the procedure, but not
section 3, the pre-merge checklist. We may be able to incorporate the
checklist into GitHub's interface using a `pull request template
<https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository>`_.
[See `this comment on GitHub issue #98
<https://github.com/Ravenbrook/mps/issues/98#issuecomment-1376945162>`__.
RB 2023-05-30]


.. _durable branch naming convention:
Expand Down Expand Up @@ -553,6 +478,7 @@ B. Document History
2023-01-23 RB_ Adding measurements.
2023-01-25 RB_ Responding to mini-review_.
2023-01-31 RB_ Adding instructions for recording the merge.
2023-05-30 RB_ Perforce Divorce: removing synchronisation with Perforce repository.
========== ===== ==================================================

.. _RB: mailto:[email protected]
Expand Down