are three times as likely to be picked as other reviewers.
are three times as likely to be picked as other reviewers.
3. It always picks the same reviewers and maintainers for the same
1. It always picks the same reviewers and maintainers for the same
branch name (unless their OOO status changes, as in point 1). It
branch name (unless their OOO status changes, as in point 1). It
removes leading `ce-` and `ee-`, and trailing `-ce` and `-ee`, so
removes leading `ce-` and `ee-`, and trailing `-ce` and `-ee`, so
that it can be stable for backport branches.
that it can be stable for backport branches.
...
@@ -58,20 +58,20 @@ As described in the section on the responsibility of the maintainer below, you
...
@@ -58,20 +58,20 @@ As described in the section on the responsibility of the maintainer below, you
are recommended to get your merge request approved and merged by maintainer(s)
are recommended to get your merge request approved and merged by maintainer(s)
from teams other than your own.
from teams other than your own.
1. If your merge request includes backend changes [^1], it must be
1. If your merge request includes backend changes [^1], it must be
**approved by a [backend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_backend)**.
**approved by a [backend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_backend)**.
1. If your merge request includes database migrations or changes to expensive queries [^2], it must be
1. If your merge request includes database migrations or changes to expensive queries [^2], it must be
**approved by a [database maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_database)**.
**approved by a [database maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_database)**.
1. If your merge request includes frontend changes [^1], it must be
1. If your merge request includes frontend changes [^1], it must be
**approved by a [frontend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_frontend)**.
**approved by a [frontend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_frontend)**.
1. If your merge request includes UX changes [^1], it must be
1. If your merge request includes UX changes [^1], it must be
**approved by a [UX team member][team]**.
**approved by a [UX team member][team]**.
1. If your merge request includes adding a new JavaScript library [^1], it must be
1. If your merge request includes adding a new JavaScript library [^1], it must be
**approved by a [frontend lead][team]**.
**approved by a [frontend lead][team]**.
1. If your merge request includes adding a new UI/UX paradigm [^1], it must be
1. If your merge request includes adding a new UI/UX paradigm [^1], it must be
**approved by a [UX lead][team]**.
**approved by a [UX lead][team]**.
1. If your merge request includes a new dependency or a filesystem change, it must be
1. If your merge request includes a new dependency or a filesystem change, it must be
**approved by a [Distribution team member][team]**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/dev-backend/distribution/) for more details.
**approved by a [Distribution team member][team]**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/dev-backend/distribution/) for more details.
@@ -121,27 +121,27 @@ All reviewers can help ensure accuracy, clarity, completeness, and adherence to
...
@@ -121,27 +121,27 @@ All reviewers can help ensure accuracy, clarity, completeness, and adherence to
-**Prior to merging**, documentation changes committed by the developer must be reviewed by:
-**Prior to merging**, documentation changes committed by the developer must be reviewed by:
1.**The code reviewer** for the MR, to confirm accuracy, clarity, and completeness.
1.**The code reviewer** for the MR, to confirm accuracy, clarity, and completeness.
1. Optionally: Others involved in the work, such as other devs or the PM.
1. Optionally: Others involved in the work, such as other devs or the PM.
1. Optionally: The technical writer for the DevOps stage. If not prior to merging, the technical writer will review after the merge.
1. Optionally: The technical writer for the DevOps stage. If not prior to merging, the technical writer will review after the merge.
This helps us ensure that the developer has time to merge good content by the freeze, and that it can be further refined by the release, if needed.
This helps us ensure that the developer has time to merge good content by the freeze, and that it can be further refined by the release, if needed.
- To decide whether to request this review before the merge, consider the amount of time left before the code freeze, the size of the change,
- To decide whether to request this review before the merge, consider the amount of time left before the code freeze, the size of the change,
and your degree of confidence in having users of an RC use your docs as written.
and your degree of confidence in having users of an RC use your docs as written.
- Pre-merge tech writer reviews should be most common when the code is complete well in advance of the freeze and/or for larger documentation changes.
- Pre-merge tech writer reviews should be most common when the code is complete well in advance of the freeze and/or for larger documentation changes.
- You can request a review and if there is not sufficient time to complete it prior to the freeze,
- You can request a review and if there is not sufficient time to complete it prior to the freeze,
the maintainer can merge the current doc changes (if complete) and create a follow-up doc review issue.
the maintainer can merge the current doc changes (if complete) and create a follow-up doc review issue.
- The technical writer can also help decide what docs to merge before the freeze and whether to work on further changes in a follow up MR.
- The technical writer can also help decide what docs to merge before the freeze and whether to work on further changes in a follow up MR.
-**To request a pre-merge technical writer review**, assign the writer listed for the applicable [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages).
-**To request a pre-merge technical writer review**, assign the writer listed for the applicable [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages).
-**To request a post-merge technical writer review**, [create an issue for one using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-ce/issues/new?issuable_template=Doc%20Review) and link it from the MR that makes the doc change.
-**To request a post-merge technical writer review**, [create an issue for one using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-ce/issues/new?issuable_template=Doc%20Review) and link it from the MR that makes the doc change.
1.**The maintainer** who is assigned to merge the MR, to verify clarity, completeness, and quality, to the best of their ability.
1.**The maintainer** who is assigned to merge the MR, to verify clarity, completeness, and quality, to the best of their ability.
- Upon merging, if a technical writer review has not been performed and there is not yet a linked issue for a follow-up review, the maintainer should [create an issue using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-ce/issues/new?issuable_template=Doc%20Review), link it from the MR, and
- Upon merging, if a technical writer review has not been performed and there is not yet a linked issue for a follow-up review, the maintainer should [create an issue using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-ce/issues/new?issuable_template=Doc%20Review), link it from the MR, and
mention the original MR author in the new issue. Alternatively, the maintainer can ask the MR author to create and link this issue before the MR is merged.
mention the original MR author in the new issue. Alternatively, the maintainer can ask the MR author to create and link this issue before the MR is merged.
- After merging, documentation changes are reviewed by:
- After merging, documentation changes are reviewed by:
1. The technical writer--**if** their review was not performed prior to the merge.
1. The technical writer -- **if** their review was not performed prior to the merge.
2. Optionally: by the PM (for accuracy and to ensure it's consistent with the vision for how the product will be used).
1. Optionally: by the PM (for accuracy and to ensure it's consistent with the vision for how the product will be used).
Any party can raise the item to the PM for review at any point: the dev, the technical writer, or the PM, who can request/plan a review at the outset.
Any party can raise the item to the PM for review at any point: the dev, the technical writer, or the PM, who can request/plan a review at the outset.
1.[Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps)
1.[Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps),
, e.g. `review-qa-raise-e-12chm0`.
e.g. `review-qa-raise-e-12chm0`.
1. Find and open the `task-runner` Deployment, e.g. `review-qa-raise-e-12chm0-task-runner`.
1. Find and open the `task-runner` Deployment, e.g. `review-qa-raise-e-12chm0-task-runner`.
1. Click on the Pod in the "Managed pods" section, e.g. `review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz`.
1. Click on the Pod in the "Managed pods" section, e.g. `review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz`.
1. Click on the `KUBECTL` dropdown, then `Exec` -> `task-runner`.
1. Click on the `KUBECTL` dropdown, then `Exec` -> `task-runner`.
...
@@ -196,7 +196,7 @@ For the record, the debugging steps to find out this issue were:
...
@@ -196,7 +196,7 @@ For the record, the debugging steps to find out this issue were:
1.`kubectl describe pod <pod name>` & confirm exact error message
1.`kubectl describe pod <pod name>` & confirm exact error message
1. Web search for exact error message, following rabbit hole to [a relevant kubernetes bug report](https://github.com/kubernetes/kubernetes/issues/57345)
1. Web search for exact error message, following rabbit hole to [a relevant kubernetes bug report](https://github.com/kubernetes/kubernetes/issues/57345)
1. Access the node over SSH via the GCP console (**Computer Engine > VM
1. Access the node over SSH via the GCP console (**Computer Engine > VM
instances** then click the "SSH" button for the node where the `dns-gitlab-review-app-external-dns` pod runs)
instances** then click the "SSH" button for the node where the `dns-gitlab-review-app-external-dns` pod runs)
1. In the node: `systemctl --version` => systemd 232
1. In the node: `systemctl --version` => systemd 232
1. Gather some more information:
1. Gather some more information:
-`mount | grep kube | wc -l` => e.g. 290
-`mount | grep kube | wc -l` => e.g. 290
...
@@ -211,7 +211,7 @@ For the record, the debugging steps to find out this issue were:
...
@@ -211,7 +211,7 @@ For the record, the debugging steps to find out this issue were:
To resolve the problem, we needed to (forcibly) drain some nodes:
To resolve the problem, we needed to (forcibly) drain some nodes:
1. Try a normal drain on the node where the `dns-gitlab-review-app-external-dns`
1. Try a normal drain on the node where the `dns-gitlab-review-app-external-dns`
pod runs so that Kubernetes automatically move it to another node: `kubectl drain NODE_NAME`
pod runs so that Kubernetes automatically move it to another node: `kubectl drain NODE_NAME`
1. If that doesn't work, you can also perform a forcible "drain" the node by removing all pods: `kubectl delete pods --field-selector=spec.nodeName=NODE_NAME`
1. If that doesn't work, you can also perform a forcible "drain" the node by removing all pods: `kubectl delete pods --field-selector=spec.nodeName=NODE_NAME`
1. In the node:
1. In the node:
- Perform `systemctl daemon-reload` to remove the dead/inactive units
- Perform `systemctl daemon-reload` to remove the dead/inactive units