| name | code-review-workflow |
| description | Provides guidance and best practices on Gerrit submission labeling, CI builder execution, Python code formatting/linting, commit metadata standardization, and testing strategy in git-repo. |
Code Review Workflow Engineering Guide
Executive Summary
Welcome to the authoritative engineering guide for the Code Review Workflow.
This living repository exists to capture critical folk knowledge, prevent the
recurrence of historical failure modes, and enforce strict architectural and
procedural boundaries across our integration pipeline. By standardizing these
protocols, we ensure high development velocity while maintaining rock-solid
codebase stability and traceability.
This guide covers the complete lifecycle of a change list (CL) from local
development to automated submission. It defines the strict Gerrit labeling
mechanisms required to trigger the Commit-Queue, mandates comprehensive CI
builder environment checks, and enforces centralized Python static analysis.
Furthermore, it outlines uncompromising standards for atomic commit metadata and
pragmatic testing state isolation to guarantee that every integration is fully
bisectable and verifiable.
For incoming engineers, adherence to these mandates eliminates the friction of
stalled pipelines, unreviewable monolithic changes, and silent CI regressions.
Treat this guide as your primary roadmap for navigating the repository's strict
submission requirements, enabling seamless transitions from peer approval to
successfully integrated code.
Summary
| Chapter Theme / Title | Scope & Objective |
|---|
| **Gerrit Submission and Labeling | Dictates strict access controls, review |
| : Workflow** : enforcement protocols, and Gerrit : | |
| : : labeling mechanisms required to advance : | |
| : : changes through the CI pipeline, ensuring : | |
| : : seamless transitions to automated : | |
| : : integration via the Commit-Queue. : | |
| **CI Builder Environment and | Defines guidelines for ensuring build |
| : Execution Integrity** : script resilience against missing : | |
| : : dependencies and managing process : | |
| : : execution contexts within LUCI and local : | |
| : : testing environments to prevent silent : | |
| : : builder failures. : | |
| **Python Code Formatting and | Governs the automated enforcement of |
| : Linting** : Python style guidelines, mandating strict : | |
| : : PEP-8 compliance, import sorting, and : | |
| : : consistent string quoting to ensure : | |
| : : codebase uniformity and prevent CI : | |
| : : regressions. : | |
| **Commit Metadata and History | Establishes the structural composition |
| : Standardization** : and metadata formatting of change lists : | |
| : : (CLs) to ensure precise issue tracker : | |
| : : integration, reliable CI/CD parsing, and : | |
| : : an atomic, bisectable repository history. : | |
| **Testing Strategy and State | Outlines test implementation boundaries, |
| : Isolation** : emphasizing pragmatic mocking limits to : | |
| : : prevent false positives and detailing : | |
| : : acceptable workflows for deferred test : | |
| : : coverage while maintaining verification : | |
| : : integrity. : | |
Chapter: Gerrit Submission and Labeling Workflow
Context: This domain dictates the strict access controls, review enforcement
protocols, and specific Gerrit labeling mechanisms required to advance changes
through the CI pipeline. Adherence ensures seamless transitions from peer
approval to automated integration via the Commit-Queue.
Summary
| Rule ID | Principle / Constraint | Priority | Primary Symptom / |
: : : : Trap :
| :-------- | :------------------------------ | :------- | :----------------- |
| T1-01 | Explicit Labeling for Gerrit | High | Leaving a change |
: : Automated Submission : : idle after :
: : : : addressing :
: : : : comments or :
: : : : receiving a :
: : : : reviewer's LGTM, :
: : : : expecting the :
: : : : reviewer to merge :
: : : : it. :
| T1-02 | Automated Submission via | Medium | Requesting a |
: : Commit-Queue (CQ) : : manual push or :
: : : : direct submit from :
: : : : repository :
: : : : maintainers after :
: : : : receiving code :
: : : : review approval. :
| T1-03 | Gerrit Trusted Contributor | Medium | Relying on a |
: : Review Enforcement Verification : : standard +2 vote :
: : : : from a non-trusted :
: : : : contributor to :
: : : : fulfill strict :
: : : : Review-Enforcement :
: : : : requirements. :
| T1-04 | Mandatory Gerrit Labels for | High | Acknowledging an |
: : Automated Submission : : approval but :
: : : : failing to apply :
: : : : the appropriate :
: : : : Gerrit labels to :
: : : : initiate the merge :
: : : : pipeline. :
| T1-05 | Gerrit Automated Submission | Medium | Leaving an |
: : Triggers : : approved patchset :
: : : : idle and waiting :
: : : : for maintainers to :
: : : : manually merge it. :
| T1-06 | Active Reviewer Rerouting for | Medium | Waiting weeks or |
: : Stalled Changes : : months for an :
: : : : inactive or OOO :
: : : : reviewer to :
: : : : respond to a :
: : : : patchset update. :
Rules
T1-01: Explicit Labeling for Gerrit Automated Submission
Rule: Always apply Verified+1 and Commit-Queue+2 explicitly to trigger
the final submission phase. Never assume a code approval automatically
initiates the pipeline.
What: Changes are not merged automatically upon receiving approval;
contributors must explicitly set the Verified+1 and Commit-Queue+2 labels
to trigger the final submission phase.
Applies To: Gerrit review UI and change submission pipeline as defined in
CONTRIBUTING.md.
Why: Contributors often mistakenly assume an LGTM implies an immediate
merge, leading to stalled changes. The project relies on explicitly triggering
the Commit-Queue to finalize CI checks and perform the merge. Failing to
adhere to this typically results in Stalled Submission Pipeline.
Trap 1: Leaving a change idle after addressing comments or receiving a
reviewer's LGTM, expecting the reviewer to merge it.
Don't:
- Waiting indefinitely after reviewer posts 'LGTM'.
Do:
- Vote
Verified+1 and Commit-Queue+2 manually to submit the change to the
automated queue.
Exceptions: Contributors lacking trusted permissions must ping a repository
maintainer to apply the final Commit-Queue+2 vote.
T1-02: Automated Submission via Commit-Queue (CQ)
Rule: Must utilize the Gerrit Commit-Queue (CQ) labeling system to merge
code. Maintainers must never perform direct manual submissions.
What: Merging code must be triggered via the Gerrit Commit-Queue (CQ)
labeling system rather than relying on direct manual submission by
maintainers.
Applies To: Gerrit code review UI and CI/CD submission workflow.
Why: Contributors would request maintainers to directly merge patches once
approved, bypassing the automated commit-queue pipeline, which guarantees that
final integration tests pass before pushing to the target branch. Failing to
adhere to this typically results in Bypassed CI / Direct Submit.
Trap 1: Requesting a manual push or direct submit from repository maintainers
after receiving code review approval.
Don't:
- Leaving a comment: "I believe everything is ready for integrating this. So
if either of you can submit it, it would be appreciated."
Do:
- Applying the
Commit-Queue+2 (CQ+2) label in Gerrit, which delegates
testing and the final merge operation to the automated bot.
T1-03: Gerrit Trusted Contributor Review Enforcement Verification
Rule: Verify review enforcement requirements are satisfied by contributors
within the explicitly configured trusted group. Never cast misleading +2 votes
if you lack valid trusted group privileges.
What: Gerrit submission requirements may mandate specific approval levels
(e.g., two trusted contributors). Votes from users with +2 access who are not
in the designated 'trusted' group do not satisfy the 'Review-Enforcement'
submit requirement.
Applies To: Gerrit repository administration and code review voting
workflows.
Why: Non-trusted contributors with +2 rights were casting +2 votes on
changes. These votes did not fulfill the 'Two trusted contributors'
Review-Enforcement requirement, leading to stalled submissions and confusion
regarding why the UI showed a +2 but blocked submission. Failing to adhere to
this typically results in Blocked Submission / Silent Requirement Failure.
Trap 1: Relying on a standard +2 vote from a non-trusted contributor to
fulfill strict Review-Enforcement requirements.
Don't:
- Leaving a +2 vote on a change as a non-trusted contributor, creating the
false appearance that the Review-Enforcement requirement has been partially
or fully met.
Do:
- Verifying the reviewer is in the explicitly configured trusted group for the
repository. If not, the reviewer should manually downgrade their invalid +2
vote to a +1 to clearly indicate that their vote does not count toward the
enforcement threshold.
Exceptions: Repositories where specific non-employee groups have been
explicitly added to the trusted administrators list.
T1-04: Mandatory Gerrit Labels for Automated Submission
Rule: Always apply Verified+1 and Commit-Queue+2 labels to initiate
the CI merge process. Never leave an approved CL in a technically unlabeled
state.
What: A code change must receive explicit Verified+1 and
Commit-Queue+2 labels by the author or reviewer to trigger the automated CI
merge process.
Applies To: Gerrit workflow / Merge execution phase.
Why: Historically, leaving a Change List (CL) in an approved but unlabeled
state causes the integration pipeline to stall indefinitely, requiring manual
intervention or reviewer pinging to trigger the CI queue. Failing to adhere to
this typically results in Merge Pipeline Stall.
Trap 1: Acknowledging an approval but failing to apply the appropriate Gerrit
labels to initiate the merge pipeline.
Don't:
- Leaving the CL in an approved state and waiting for auto-submission without
applying the
Verified+1 or Commit-Queue+2 labels.
Do:
- Explicitly applying
Verified+1 (and Commit-Queue+2 if ready) once
reviewers have approved the logic, to instruct the automation to merge the
code.
T1-05: Gerrit Automated Submission Triggers
Rule: Must actively signal patch readiness to Gerrit systems using proper
label thresholds. Avoid leaving patchsets idle assuming upstream maintainer
action.
What: A patchset requires specific label thresholds ('Verified+1' and
'Commit-Queue+2') to trigger automated submission in the Gerrit workflow.
Applies To: Gerrit review UI and automated CI/CD submission process for
the git-repo codebase.
Why: Contributors frequently asked how to integrate changes after
receiving an approval, leading to stalled patches because the automated
pipeline was not explicitly triggered. Failing to adhere to this typically
results in Stalled Patch Integration.
Trap 1: Leaving an approved patchset idle and waiting for maintainers to
manually merge it.
Don't:
- Waiting indefinitely after receiving an 'LGTM' without setting workflow
labels.
Do:
- The patch author manually sets the 'Verified' flag (if locally tested) and
applies the 'Commit-Queue+2' vote to signal readiness for automated merge.
