-
Notifications
You must be signed in to change notification settings - Fork 115
Robottelo Reviewers Guide
This doc aims to plot and explain the reviewing, approving, and merging process in SatelliteQEs Robottelo Github repository. The process for GitHub repositories, specifically tailored for teams working in a collaborative environment. The review and approval workflow on GitHub ensures that all changes are thoroughly examined, discussed, and refined before they are merged into the main codebase.
By following a structured reviewing and approving process, teams can enhance collaboration, ensure code quality, and maintain a healthy codebase. This guide will delve into each aspect of the process, providing best practices and practical tips to help your team navigate the GitHub workflow effectively.
Code reviews and reviewers are critical for maintaining code quality. They involve detailed inspection of the proposed changes by one or more team members, who provide feedback, suggest improvements, and ensure adherence to coding standards.
- Team Code Reviewers - Those who review the code from their own (but not limited to) team.
- Tier 1 Code Reviewers - Those who possess the knowledge of components across teams and the robottelo automation framework.
- Tier 2 Code Reviewers - Those who possess the knowledge of components and are good in the robottelo automation framework and external tools used in the framework. Also knows about the processes in the framework.
The codeowners file in the root directory of robottelo lists the ownership of test modules for each Team in Satellite QE. When any Pull Request touches the test module for a team, all the team members in that team will be invited for the review automatically. Any Team members review and approval would be considered as one of two ACKs needed for PR merge.
-
Component Changes - The code changes in the tests directory where all component test modules are located.
-
Frameowrk Changes - The code changes in entire robottelo framework other than tests directory.
Integrating automated reviews within the review process helps in automated fashioned approvals and merged in some cases. Continuous Integration (CI) tools can automatically run tests on the proposed changes, ensuring they do not break existing functionality.
Robottelo has a GitOps deployed that helps in automatic reviewing and automatic merging of a Pull Request. But only in following cases:
-
Dependabot PRs - When a Pull is requested by dependabot for external python package dependencies update, an internal github action watching for the dependabot PRs shall trigger the PRT with some predefined list of tests mapped with the dependency. Once all the tests are passed, the Github Action will merge the PR in the target branch. An ACK from any reviewer is optional in this case.
-
AutoMergeCherryPicked Labeling - zStream auto-cherrrypicked PRs inherits this label from the parent PR along with PRT comment. A Github Action triggers the same PRT tests on the auto-cherrypicked zStream PR and when all test passes, the Github Action automatically merges the PR in target branch. An ACK from any reviewer is optional in this case.
Robottelo contributors are expected to adhere to the rules for approving/merging the pull requests. This can include requiring a certain number of approvals, ensuring reviews by specific team members, and integrated status checks from CI tools.
1. PR contains only component-specific changes:
- Number of ACKS: 2
- Reviewers:
- T1 Reviewer or Team Reviewer
- Optional: T2 Reviewer
2. PR contains both component and framework changes:
- Number of ACKS: 2
- Reviewers:
- T1 Reviewer or Team Reviewer
- T2 Reviewer
- Optional: None
3. PR contains only framework changes:
- Number of ACKS: 2
- Reviewers:
- T2 Reviewer
- Optional: T1 Reviewer and/or Team Reviewer
4. Cherrypicked PR with PRT passing:
- Number of ACKS: 1
- Reviewers: Based on the above scenarios
Once a pull request has been reviewed and approved, it can be merged into the main codebase. Teams can choose from different merge strategies, such as merge commits, squashing, or rebasing, depending on their preference. But in general we advise squashing
strategy for a cleaner commit history.