Skip to content

Commit

Permalink
Merge pull request #203 from TYPO3-Documentation/lwolf-core-mergers
Browse files Browse the repository at this point in the history
[FEATURE] Add a Core Mergers Section to the "Howtos"
  • Loading branch information
linawolf committed Jul 25, 2021
2 parents a6328f8 + 17896f4 commit c1db84d
Show file tree
Hide file tree
Showing 30 changed files with 355 additions and 180 deletions.
87 changes: 87 additions & 0 deletions Documentation/CoreMergers/Backport.rst
@@ -0,0 +1,87 @@
.. include:: ../Includes.txt

.. index::
single: Gerrit; Backport a Change
single: Backport a Change

.. _coreMergers-backport:
.. _lifeOfAPatch-backport:


=================
Backport a Change
=================

.. include:: /_includes/CoreMergers.rst.txt


Before you start, wait until the review process has been successfully completed for the most
recent affected branch.

Use Gerrit for the cherry-pick
==============================

First try to use the Gerrit cherry-pick feature for automatic backporting.

.. rst-class:: bignums-xxl

#. Cherry pick into a branch

.. figure:: /Images/External/Gerrit/CoreMergers/Backport1.png
:class: with-shadow

Cherry picking via Gerrit

#. Choose branch and adjust commit message

In the following modal, you can select the branch to backport. Existing
branches are shown by autocomplete.

Remove from the commit message everything below the Change-ID because the
information about former reviewers is not needed for the cherry pick. Make sure
that you **don't alter the Change-ID** but remove every line (also empty ones)
below it. After doing so hit the :guilabel:`Cherry Pick` button.


.. figure:: /Images/External/Gerrit/CoreMergers/Backport2.png
:class: with-shadow

Adjust the commit message and click :guilabel:`Cherry Pick`

#. Review backport patch

This creates a new Change on Gerrit that needs to be reviewed and
merged once more.

.. figure:: /Images/External/Gerrit/CoreMergers/Backport3.png
:class: with-shadow

A new change based on another branch is created. It has the status
"Active" once more.

#. Merge the backport

If the backport is a :ref:`Low brainer <Gerrit-low-brainers>` you can vote
:guilabel:`+2` and continue to merge the backport. Otherwise it should be
reviewed by at least one other Core Merger.


.. figure:: /Images/External/Gerrit/CoreMergers/Backport4.png
:class: with-shadow

Merging the backport

Manual backport
===============

If the automatic backport fails, you need to manually cherry-pick the patch to the target branch. (e.g. cherry-pick the
master patch onto your local (up to date) 10.4 branch) You will most likely need to adjust the code for the older branch.

Edit the commit message to comply to the guidelines again. (e.g. remove the Reviewed-* and Tested-* lines added by Gerrit)

.. important::
The Change-Id must be left unchanged, otherwise Gerrit is not able to link the backport to its original change!

Push the review back to Gerrit.

On Gerrit the original patch will show the cherry-pick as a related patch.
18 changes: 18 additions & 0 deletions Documentation/CoreMergers/Index.rst
@@ -0,0 +1,18 @@
.. include:: ../Includes.txt

.. highlight:: bash

.. _core_mergers:

============
Core Mergers
============


.. toctree::
:titlesonly:

Review
Merge
Backport
Revert
64 changes: 64 additions & 0 deletions Documentation/CoreMergers/Merge.rst
@@ -0,0 +1,64 @@
.. include:: ../Includes.txt

.. index::
single: Gerrit; Submit a Change
single: Submit a Change

.. _coreMergers-merge:
.. _lifeOfAPatch-Merging-Patches:

==============
Merge patches
==============

.. include:: /_includes/CoreMergers.rst.txt


.. _lifeOfAPatch-Merging-Patches-Prerequisites:

The merging process
===================


.. rst-class:: bignums-xxl

#. Prerequisites

A patch can only be merged if it has received a :ref:`+ 2<Gerrit-Vote-plus-2>`
vote from a Core Merger. A :guilabel:`+2` can be given by the second
Core Merger to review the patch positively.

Merging gets blocked by a :guilabel:`-2` vote.

#. Submit the patch

You can merge by clicking the :guilabel:`Submit` button. The Core Merger
to merge the patch also has to do
the :ref:`Backporting <coreMergers-backport>` if the submit-message requests
for a backport.

.. figure:: /Images/External/Gerrit/CoreMergers/Merging1.png
:class: with-shadow

The :guilabel:`Submit` button on a patch in status :guilabel:`Ready
to submit`.

#. The patch is successfully merged

When the merging is successful, the Change looks like this:

.. figure:: /Images/External/Gerrit/CoreMergers/Merging2.png
:class: with-shadow

The patch in status :guilabel:`Merged`.

#. Backport if necessary

If required in the commit message, do the
:ref:`backport <coreMergers-backport>` next.

#. How to revert a submitted change

There is a :guilabel:`Revert` button available now. Please stick to the
:ref:`revert workflow<coreMergers-revert>` if reverting a change should
become necessary.
46 changes: 46 additions & 0 deletions Documentation/CoreMergers/Revert.rst
@@ -0,0 +1,46 @@
.. include:: ../Includes.txt

.. index::
single: Gerrit; Revert a Change
single: Revert a Change

.. _coreMergers-revert:
.. _lifeOfAPatch-Reverting-Patches:

==============
Revert patches
==============

.. include:: /_includes/CoreMergers.rst.txt

It is important to have traceable code. This means that any person inspecting the bug tracker, Gerrit changes or the
commit log shall be able to trace if a patch has been reverted. Therefore we must add this information to all affected
places.

If there's the need to revert a patch, please stick to the following rules:

#. Create a ticket on Forge for the revert, if there's not yet a bug report. (Maybe re-use the original ticket,
if you re-push the patch again with the original ticket number.)

#. Visit the original ticket in Forge and

#. link it to the revert ticket

#. add a comment to the ticket with information that it is reverted

#. set the Status from Resolved to Accepted

#. Use the Revert button in Gerrit

#. Modify the commit message of the newly created patch to contain

#. the commit hash as generated by Gerrit

#. a description why the revert is needed

#. a Releases-line that contains all releases where the original patch was merged (check if the backports where
really merged)

#. a Resolves-line for the revert ticket as created above (you can skip this if you re-used the original ticket)

#. a Reverts-line for the ticket of the original patch
91 changes: 91 additions & 0 deletions Documentation/CoreMergers/Review.rst
@@ -0,0 +1,91 @@
.. include:: ../Includes.txt

.. index::
single: Gerrit; Submit a Change
single: Submit a Change

.. _coreMergers-review:

===============================
Review a patch as a Core Merger
===============================

.. include:: /_includes/CoreMergers.rst.txt

Please see :ref:`reviewPatch` for general information on how to review a
patch.

.. figure:: /Images/External/Gerrit/CoreMergers/VoteCoreMerger.png
:class: with-shadow

Vote by clicking the :guilabel:`Reply` button


.. _Gerrit-Vote-plus-1:

Voting +1
=========

.. _Gerrit-Practical-considerations:

Practical considerations
------------------------

The Core Merger who gave an early +1 should try and go back to
transform the +1 into a +2
after a second review came in, if applicable.

Each newly pushed patch requires a complete new round of voting before it can
be submitted. So everyone that reviewed once is invited to re-vote as soon as
a new patch is pushed. Using Gerrit's Patch History feature allows to quickly
see what has changed from the already reviewed patch to the new one.

Consider these rules when comparing patches:

* If the patch was re-pushed due to the comments, check the diff between the
versions of the patch.
* If the patch needed to be rebased onto current master, the change set might
contain the changes due to rebasing. So better check the diff between base
and most recent version in this case.

.. _Gerrit-Vote-plus-2:

Voting +2
=========

.. figure:: /Images/External/Gerrit/CoreMergers/Vote-plus-2.png
:class: with-shadow

Ready to merge
--------------

A Core Merger can give a +2 vote on :pn:`Code Review` and :pn:`Verified` if
there is already a positive vote by another Core Merger.

.. _Gerrit-No-brainers:
.. _Gerrit-Low-brainers:

Low brainers
-------------

A Core Merger can give a +2 and submit right away in case of "low-brainers"
(what used to be called "FYI").

A Core Merger can give a +2 and wait a bit before submitting
(used to be called "FYI24", "FYI48", ...).

.. _Gerrit-Vote-minus-2:

Voting -2
=========

.. figure:: /Images/External/Gerrit/CoreMergers/Vote-minus-2.png
:class: with-shadow

A :guilabel:`-2` vote by a Core Merger blocks a patch from merging.
In contrast to -1, a -2 is a **veto**. Use with care. With a -2, you
are taking responsibility of the patch and basically stating that it will not
be done until you actively remove your negative vote.

To date, we have had no real problem with someone giving -2 and then
not acting responsibly. It would be great if it stays this way.
53 changes: 2 additions & 51 deletions Documentation/HandlingAPatch/Backporting.rst
@@ -1,52 +1,3 @@
.. include:: ../Includes.txt
:orphan:

.. index::
single: Gerrit; Backport a Change
single: Backport a Change

.. _lifeOfAPatch-backport:

===================================
Backport a change to other branches
===================================

.. note::

This is an informative section targeted towards members of the core team only.


First wait until the review process was successfully completed for the most recent affected branch.

Use Gerrit for the cherry-pick
==============================

First try to use the Gerrit cherry-pick feature for automatic backporting.

.. image:: _assets/gerrit_cherrypick_1.png
:width: 400px

In the following modal, you can easily select the branch, you want to cherry pick to, by just typing partial information.

.. image:: _assets/gerrit_cherrypick_2.png
:width: 400px

Remove from the commit message everything below the Change-ID because the information about former reviewers is not needed
for the cherry pick. Make sure that you **don't alter the Change-ID** but remove every line (also empty ones) below it. After doing so hit the "Cherry Pick Change" button.

.. image:: _assets/gerrit_cherrypick_3.png
:width: 400px

Manual backport
===============

If the automatic backporting fails, you need to manually cherry-pick the patch to the target branch. (e.g. cherry-pick the
master patch onto your local (up to date) TYPO3_6-2 branch) You will most likely need to adjust the code for the older branch.

Edit the commit message to comply to the guidelines again. (e.g. remove the Reviewed-* and Tested-* lines added by Gerrit)

.. important::
The Change-Id must be left unchanged, otherwise Gerrit is not able to link the backport to its original change!

Push the review back to Gerrit.

On Gerrit the original patch will show the cherry-pick as a related patch.
The page was moved to: :ref:`coreMergers-backport`
2 changes: 0 additions & 2 deletions Documentation/HandlingAPatch/Index.rst
Expand Up @@ -51,7 +51,5 @@ automatically run on every patch (set), the results being shown as a +1 or
Review
Rebase
ResolveMergeConflicts
Backporting
Revert


0 comments on commit c1db84d

Please sign in to comment.