In large-scale software projects, the efficiency and trustworthiness of the code review process are crucial. Recent analysis of over 200 patch tickets has revealed a significant disconnect between patch testing and committer actions. Addressing this gap can streamline workflows, enhance collaboration, and improve the overall quality of code deployments.
Understanding the Current Workflow
The existing workflow relies on the “dev-feedback” keyword, which signals to committers that a patch is ready for review after testing. While this method is intended to facilitate communication, it presents several challenges:
- Most patch testers are not code reviewers. They follow reproduction steps but rarely assess the patch’s code quality.
- Many patches require substantial changes or complete rewrites, making initial test reports less valuable if not accompanied by a code review.
- Committers often disregard patch testing reports unless they come from trusted sources—individuals known for thorough code reviews.
This dynamic creates a trust-based bottleneck, rendering many patch testing reports ineffective. Moreover, testers with established reputations are expected to both test and review code, further complicating the process.
Why Patch Testing Alone Matters
Patch testing is fundamentally a negative-conditional process. Its primary goal is to identify faulty patches and request changes or updates. This triage step helps filter out problematic patches, a task that can be performed efficiently even by non-technical contributors.
In contrast, code reviewing is a complex task requiring experience and a deep understanding of the codebase. While patch testing can catch obvious issues, code reviews ensure the solution is robust and aligns with project standards.
The Core Problem
Currently, there is no dedicated mechanism to alert core developers after a code review, apart from using the “dev-feedback” keyword. According to project guidelines, “dev-feedback” is meant for scenarios where a core developer’s response is needed, such as double sign-offs for backporting changes during release candidates.
However, using “dev-feedback” for code review notifications blurs its intended purpose. This leads to three common scenarios:
- Non-technical patch tester: Adds “dev-feedback,” prompting the committer to re-review everything due to lack of trust.
- Technical patch tester after a full code review: Adds “dev-feedback,” allowing the committer to proceed confidently.
- Technical patch tester without a full code review: Adds “dev-feedback,” but the patch may not be ready, potentially harming the tester’s reputation.
This ambiguity forces committers to approach each report with caution, often leading them to repeat work already documented by others. This redundancy is a symptom of a system where test reports are frequently ignored due to historical unreliability.
A Proposed Solution: Introducing a New Workflow Keyword
Two solutions have been considered:
- Temporary fix: Commenting within the report that no code review has been conducted. However, this can be easily missed and is not filterable in report listings.
- Optimal fix: Introducing a new workflow keyword, “needs-code-review.”
Ideal Workflow with ‘needs-code-review’
With the addition of “needs-code-review,” the workflow can be significantly improved:
- Non-technical patch testers: After testing, set the “needs-code-review” keyword. Code reviewers can then filter and address these reports, enhancing their quality before reaching committers.
- Technical patch testers performing full code reviews: Continue using “dev-feedback” as before.
- Technical patch testers not reviewing code: Set “needs-code-review,” allowing another reviewer to handle the code assessment.
Advantages of the New Approach
- Committers can prioritize “dev-feedback” reports, accelerating patch processing and reducing the need to start from scratch.
- Code reviewers can focus on “needs-code-review” tickets, supporting committers and improving report quality.
- This system encourages double sign-offs, as non-committers can contribute to code reviews, making the process more collaborative.
- Code reviewing becomes a more accessible activity during team contribution days.
- Patch testing and code reviewing become distinct processes, allowing non-technical users to participate without being burdened by code quality concerns.
- Training for non-technical users can focus on patch testing, broadening participation and enabling batch testing.
- Technical testers are not always expected to conduct full code reviews, streamlining the triage process for the core test team.
Conclusion
By consistently reserving the “dev-feedback” keyword for its intended purposes—such as facilitating trust-based commit pushes, supporting backporting, and administrative notifications—the workflow becomes more transparent and efficient. Introducing a “needs-code-review” keyword bridges the gap between testing and code review, ensuring that patches are properly vetted and that all contributors, regardless of technical expertise, can play a meaningful role in the development process.
Read more such articles from our Newsletter here.