Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve code review management for pull requests #2752

Closed
antoninbas opened this issue Sep 10, 2021 · 1 comment
Closed

Improve code review management for pull requests #2752

antoninbas opened this issue Sep 10, 2021 · 1 comment
Assignees
@antoninbas
Labels
kind/task Categorizes issue or PR as related to a routine task that needs to be performed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@antoninbas
Copy link
Contributor

We have decided that some steps were needed to help ensure that pull requests are reviewed more actively and in a more timely fashion, by the right set of people.

Therefore we are planning to introduce a mapping between the different "area labels" (Github labels which describe different areas of the code base, or different features) and the most appropriate "reviewers" (identified by their Github ids) for pull requests which modify code in the areas identified by these labels. This (YAML) mapping will be checked into this repository, and consumed by a Github action which will run every time a pull request is updated. The action will look at the area labels for the pull request, and based on the mapping information, it will request reviews from the "right" people (of course, everyone is still welcome to review any PR).

In addition to the "reviewers", the mapping will also define "approvers" for each area label. All approvers are also reviewers for their given label(s). When a pull request receives enough approving reviews from members with the "approver" role, the PR will be labelled as "ready to merge" and maintainers can proceed with merging the PR. Our long-term hope with this is that the responsibility of determining whether a pull request is ready to be merged will be shared across a larger number of Antrea contributors.

A few other pieces of information:

  • any member of the antrea-io organization is welcome to open a PR to add themselves as a "reviewer" for a given set of area labels
  • a "reviewer" can be promoted to "approver" by lazy consensus among maintainers (exact process will need to be documented in GOVERNANCE document)

Contributors are highly encouraged to label their own PRs (as well as each other's PRs) with the correct area labels, as this is a requirement for this process to work appropriately. Longer-term, the labelling process may be partially automated based on the set of files modified by a PR. The current set of labels may be a bit out-of-date, so we are planning to refine it over time.

The Github action is customizable. At first, we will be requiring 1 approving review for each area label, and at least 2 approving reviews overall, in order to consider a PR as "ready to merge".

We are planning to roll out this change progressively over the next couple of weeks.
@antoninbas antoninbas added the kind/task Categorizes issue or PR as related to a routine task that needs to be performed label Sep 10, 2021
@antoninbas antoninbas self-assigned this Sep 10, 2021
antoninbas added a commit to antoninbas/antrea that referenced this issue Sep 22, 2021
@antoninbas
Use @antrea-io/review-manager to manage PR reviews
5a7ba89 
Reviews will be automatically requested based on the area label(s) added
to the PR (at least one area label if required for each PR). Based on
the set of approving reviews, the Github action will determine whether
the PR can be merged.

For the sake of initial testing, the action will only run on PRs
labelled with 'review-manager-test'.

One potential concern is rate-limiting of Github API operations (1,000
requests per hour per repository). We will see if this becomes an issue
and we can adjust @antrea-io/review-manager accordingly if needed.

See antrea-io#2752

Signed-off-by: Antonin Bas <[email protected]>
@antoninbas antoninbas mentioned this issue Sep 22, 2021
Merged
antoninbas added a commit to antoninbas/antrea that referenced this issue Sep 22, 2021
@antoninbas
Use @antrea-io/review-manager to manage PR reviews
d604e96 
Reviews will be automatically requested based on the area label(s) added
to the PR. Based on the set of approving reviews, the Github action will
then determine whether the PR can be merged.

For the sake of initial testing, the action will only run on PRs
labelled with 'review-manager-test'.

One potential concern is rate-limiting of Github API operations (1,000
requests per hour per repository). We will see if this becomes an issue
and we can adjust @antrea-io/review-manager accordingly if needed.

See antrea-io#2752

Signed-off-by: Antonin Bas <[email protected]>
antoninbas added a commit to antoninbas/antrea that referenced this issue Sep 22, 2021
@antoninbas
Use @antrea-io/review-manager to manage PR reviews
c4bac95 
Reviews will be automatically requested based on the area label(s) added
to the PR. Based on the set of approving reviews, the Github action will
then determine whether the PR can be merged.

For the sake of initial testing, the action will only run on PRs
labelled with 'review-manager-test'.

One potential concern is rate-limiting of Github API operations (1,000
requests per hour per repository). We will see if this becomes an issue
and we can adjust @antrea-io/review-manager accordingly if needed.

See antrea-io#2752

Signed-off-by: Antonin Bas <[email protected]>
antoninbas added a commit that referenced this issue Sep 23, 2021
@antoninbas
Use @antrea-io/review-manager to manage PR reviews (#2818)
0946ca6 
Reviews will be automatically requested based on the area label(s) added
to the PR. Based on the set of approving reviews, the Github action will
then determine whether the PR can be merged.

For the sake of initial testing, the action will only run on PRs
labelled with 'review-manager-test'.

One potential concern is rate-limiting of Github API operations (1,000
requests per hour per repository). We will see if this becomes an issue
and we can adjust @antrea-io/review-manager accordingly if needed.

See #2752

Signed-off-by: Antonin Bas <[email protected]>
@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@antoninbas antoninbas

Labels
kind/task Categorizes issue or PR as related to a routine task that needs to be performed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@antoninbas