diff options
author | Craig Scott <craig.scott@crascit.com> | 2019-05-26 11:23:15 (GMT) |
---|---|---|
committer | Craig Scott <craig.scott@crascit.com> | 2019-05-30 09:45:54 (GMT) |
commit | 49f5b6f7bf426d760e6c935f6e8c308d921138a7 (patch) | |
tree | 42685f8b34a55f36e21845b1aa906ed160024a66 /Help/dev | |
parent | 63f149f5989ea4adfa522cec63a954b5059c223c (diff) | |
download | CMake-49f5b6f7bf426d760e6c935f6e8c308d921138a7.zip CMake-49f5b6f7bf426d760e6c935f6e8c308d921138a7.tar.gz CMake-49f5b6f7bf426d760e6c935f6e8c308d921138a7.tar.bz2 |
Help: Document the expire and external discussion resolve states
Our practice of closing MRs temporarily while discussion
takes place in a separate issue isn't always well understood
by MR authors. Expiring a MR seems to be better understood,
but making it clear that it is also a temporary state is helpful.
Diffstat (limited to 'Help/dev')
-rw-r--r-- | Help/dev/review.rst | 75 |
1 files changed, 68 insertions, 7 deletions
diff --git a/Help/dev/review.rst b/Help/dev/review.rst index 1d664c4..cbde6fe 100644 --- a/Help/dev/review.rst +++ b/Help/dev/review.rst @@ -377,7 +377,15 @@ command is needed to stage it again. Resolve ======= -A MR may be resolved in one of the following ways. +The workflow used by the CMake project supports a number of different +ways in which a MR can be moved to a resolved state. In addition to +the conventional practices of merging or closing a MR without merging it, +a MR can also be moved to a quasi-resolved state pending some action. +This may involve moving discussion to an issue or it may be the result of +an extended period of inactivity. These quasi-resolved states are used +to help manage the relatively large number of MRs the project receives +and are not an indication of the changes being rejected. The following +sections explain the different resolutions a MR may be given. Merge ----- @@ -433,15 +441,68 @@ Close ----- If review has concluded that the MR should not be integrated then it -may be closed through GitLab. +may be closed through GitLab. This would normally be a final state +with no expectation that the MR would be re-opened in the future. +It is also used when a MR is being superseded by another separate one, +in which case a reference to the new MR should be added to the MR being +closed. Expire ------ If progress on a MR has stalled for a while, it may be closed with a ``workflow:expired`` label and a comment indicating that the MR has -been closed due to inactivity. - -Contributors are welcome to re-open an expired MR when they are ready -to continue work. Please re-open *before* pushing an update to the -MR topic branch to ensure GitLab will still act on the association. +been closed due to inactivity (it may also be done where the MR is blocked +for an extended period by work in a different MR). This is not an +indication that there is a problem with the MR's content, it is only a +practical measure to allow the reviewers to focus attention on MRs that +are actively being worked on. As a guide, the average period of inactivity +before transitioning a MR to the expired state would be around 2 weeks, +but this may decrease to 1 week or less when there is a high number of +open merge requests. + +Reviewers would usually provide a message similar to the following when +resolving a MR as expired:: + + Closing for now. @<MR-author> please re-open when ready to continue work. + +This is to make it clear to contributors that they are welcome to re-open +the expired MR when they are ready to return to working on it and moving +it forward. In the meantime, the MR will appear as ``Closed`` in GitLab, +but it can be differentiated from permanently closed MRs by the presence +of the ``workflow:expired`` label. + +**NOTE:** Please re-open *before* pushing an update to the MR topic branch +to ensure GitLab will still act on the association. If changes are pushed +before re-opening the MR, the reviewer should initiate a ``Do: check`` to +force GitLab to act on the updates. + +External Discussion +------------------- + +In some situations, a series of comments on a MR may develop into a more +involved discussion, or it may become apparent that there are broader +discussions that need to take place before the MR can move forward in an +agreed direction. Such discussions are better suited to GitLab issues +rather than in a MR because MRs may be superseded by a different MR, or +the set of changes may evolve to look quite different to the context in +which the discussions began. When this occurs, reviewers may ask the +MR author to open an issue to discuss things there and they will transition +the MR to a resolved state with the label ``workflow:external-discussion``. +The MR will appear in GitLab as closed, but it can be differentiated from +permanently closed MRs by the presence of the ``workflow:external-discussion`` +label. Reviewers should leave a message clearly explaining the action +so that the MR author understands that the MR closure is temporary and +it is clear what actions need to happen next. The following is an example +of such a message, but it will usually be necessary to tailor the message +to the individual situation:: + + The desired behavior here looks to be more involved than first thought. + Please open an issue so we can discuss the relevant details there. + Once the path forward is clear, we can re-open this MR and continue work. + +When the discussion in the associated issue runs its course and the way +forward is clear, the MR can be re-opened again and the +``workflow:external-discussion`` label removed. Reviewers should ensure +that the issue created contains a reference to the MR so that GitLab +provides a cross-reference to link the two. |