README ¶
Reviewers and Approvers
Questions this Doc Seeks To Answer
- What do we need to get a PR merged into kubernetes/kubernetes?
- What are reviewers, approvers, and the OWNERS files?
- How does the reviewer selection mechanism work? approver selection mechanism work?
- How does an approver know which PR s/he has to approve?
Overview
Every GitHub directory which is a unit of independent code contains a file named "OWNERS". The file lists reviewers and approvers for the directory. Approvers (or previously called assignees) are owners of the codes.
Approvers:
- have contributed substantially to the repo
- can provide a final approval (
/approve
) indicating whether a change to a directory or subdirectory should be accepted - Approval is done on a per directory basis and subdirectories inherit their parents directory's approvers
Reviewers:
- generally a larger set of current and past contributors
- They are responsible for a more thorough code review, discussing the implementation details and style
- Provide an
/lgtm
when they are satisfied with the Pull Request
An example of the OWNERS file is listed below:
reviewers:
- jack
- ken
- lina
approvers:
- jack
- ken
- lina
Note that items in the OWNERS files can be GitHub usernames, or aliases defined in OWNERS_ALIASES files. An OWNERS_ALIASES file is another co-existed file that delivers a mechanism for defining groups. However, GitHub Team names are not supported. We do not use them because there is no audit log for changes to the GitHub Teams. This way we have an audit log.
Blunderbuss And Reviewers
lgtm Label
LGTM is abbreviation for "look good to me". The lgtm label is normally given when the code has been thoroughly reviewed. Getting it means the PR is one step away from getting merged. Reviewers of the PR give the label to a PR by typing /lgtm in a comment, or retract it by typing /lgtm cancel. Authors of the PR cannot give the label, but they can cancel it. The bot retracts the label automatically if someone updates the PR with a new commit.
Blunderbuss Selection Mechanism
Blunderbuss provides statistical means to select a subset of approvers found in OWNERS files for approving a PR. A PR consists of changes on one or more files, in which each file has different number of lines of codes changed. Blunderbuss determines the magnitude of code change within a PR using total number of lines of codes changed across various files. Number of reviewers selected for each PR is 2.
Algorithm for selecting reviewers is as follows:
(1) determine potential reviewers of a file by going over all reviewers found in the OWNERS files for current and parent directories of the file (deduplication involved)
(2) assign each changed file with a weightage based on number of lines of codes changed
(3) assign each potential reviewer with a weightage by summing up weightages of all changed files in which s/he is a reviewer
(4) randomly select 2 reviewers based on their weightage
Approval Handler and the Approved Label
approved Label
A PR cannot be merged into the Kubernetes repo without the approved label. In order for the approved label to be applied, every file modified by the PR must be approved (via /approve) by an approver from the OWNERs files. Note, this does not necessarily require multiple approvers. The process is best illustrated in the example below.
Approval Selection Mechanism
First, it is important to understand that ALL approvers in an OWNERS file can approve any file in that directory AND its subdirectories. Second, it is important to understand the somewhat-competing goals of the bot when selecting approvers:
-
Provide a subset of approvers that can approve all files in the PR
-
Provide a small subset of approvers and suggest the same reviewers as blunderbuss if possible (people can be both reviewers and approvers)
-
Do not always suggest the same set of people to approver and do not consistently suggest people from the root OWNERS file
The exact algorithm for selecting approvers is somewhat complex; it is an set cover approximation with consideration for existing assignees. To read it in depth, check out the approvers source code linked at the end of the README.
Example
Suppose files in directories E and G are changed in a PR created by PRAuthor. Any combination of approver(s) listed below can approve the PR in order to get it merged:
(1) approvers found in OWNERS files for leaf (current) directories E and G
(2) approvers found in OWNERS files for parent directories B and C
(3) approvers found in OWNERS files for root directory A
Note someone can be both a reviewer found in OWNERS files for directory A and E. If s/he is selected as an approver and gives approval, it approves entire PR because s/he is also a reviewer for the root directory A.
Step 1:
K8s-bot creates a comment that suggests the selected approvers and shows a list of OWNERS file(s) where the approvers can be found.
[APPROVALNOTIFIER] This PR is **NOT APPROVED**
This pull-request has been approved by: *PRAuthor*
We suggest the following additional approvers: **approver1,** **approver2**
If they are not already assigned, you can assign the PR to them by writing `/assign @approver1 @approver2` in a comment when ready.
∇ Details
Needs approval from an approver in each of these OWNERS Files:
* /A/B/E/OWNERS
* /A/B/G/OWNERS
You can indicate your approval by writing `/approve` in a comment
You can cancel your approval by writing `/approve cancel` in a comment
A selected approver such as approver1 can be notified by typing /assign @approver1
in a comment.
Step 2:
Case (a): approver1 is in the E OWNERS file. S/he writes /approve
K8s-bot updates comment:
[APPROVALNOTIFIER] This PR is **NOT APPROVED**
This pull-request has been approved by: *approver1, PRAuthor*
We suggest the following additional approver: **approver2**
If they are not already assigned, you can assign the PR to them by writing /assign @approver2 in a comment when ready.
∇ Details
Needs approval from an approver in each of these OWNERS Files:
~* /A/B/E/OWNERS~ [approver1]
* /A/B/G/OWNERS
You can indicate your approval by writing `/approve` in a comment
You can cancel your approval by writing `/approve cancel` in a comment
Case (b): approver3 is NOT an approver of the PR. S/he writes /approve
K8s-bot updates comment:
[APPROVALNOTIFIER] This PR is **NOT APPROVED**
This pull-request has been approved by:* approver3, PRAuthor*
We suggest the following additional approvers: **approver1,** **approver2**
If they are not already assigned, you can assign the PR to them by writing /assign @approver1 @approver2 in a comment when ready.
∇ Details
Needs approval from an approver in each of these OWNERS Files:
* /A/B/E/OWNERS
* /A/B/G/OWNERS
You can indicate your approval by writing `/approve` in a comment
You can cancel your approval by writing `/approve cancel` in a comment
Case (c): approver1 is an approver of the PR. S/he writes /lgtm
K8s-bot updates comment:
[APPROVALNOTIFIER] This PR is **NOT APPROVED**
This pull-request has been approved by: *approver1, PRAuthor*
We suggest the following additional approver: **approver2**
If they are not already assigned, you can assign the PR to them by writing /assign @approver2 in a comment when ready.
∇ Details
Needs approval from an approver in each of these OWNERS Files:
* ~/A/B/E/OWNERS~ [approver1]
* /A/B/G/OWNERS
You can indicate your approval by writing `/approve` in a comment
You can cancel your approval by writing `/approve cancel` in a comment
The lgtm label is immediately added to the PR.
Step 3: approver2 writes /approve
K8s-bot updates comment:
[APPROVALNOTIFIER] This PR is** APPROVED**
The following people have approved this PR:* approver1**, **approver2, PRAuthor*
∇ Details
Needs approval from an approver in each of these OWNERS Files:
* ~/A/B/E/OWNERS~ [approver1]
* ~/A/B/G/OWNERS~ [approver2]
You can indicate your approval by writing `/approve` in a comment
You can cancel your approval by writing `/approve cancel` in a comment
K8s-bot merges the PR. It needs to wait its turn in submit queue and passes tests.
Final Notes
Obtaining approvals from selected approvers is the last step towards merging a PR. The approvers approve a PR by typing /approve
in a comment, or retract it by typing /approve
cancel.
Algorithm for getting the status is as follow:
(1) run through all comments after newest commit to obtain latest intention of approvers
(2) put all approvers into an approver set
(3) determine whether a file has at least one approver in the approver set
(4) add the status to the PR if all files have been approved
If an approval is cancelled, the bot will delete the status added to the PR and remove the approver from the approver set. If someone who is not an approver in the OWNERS file types /approve
in a comment, the PR will not be approved. If someone who is an approver in the OWNERS file and s/he does not get selected, s/he can still type /approve
or /lgtm
in a comment, pushing the PR forward.
Code Implementation Links
Blunderbuss: prow/plugins/blunderbuss/blunderbuss.go
LGTM: prow/plugins/lgtm/lgtm.go
Approve: prow/plugins/approve/approve.go
Documentation ¶
Index ¶
- Constants
- func GenerateTemplate(templ, name string, data interface{}) (string, error)
- func GetMessage(ap Approvers, linkURL *url.URL, org, repo, branch string, usePrefix bool, ...) *string
- func IntersectSetsCase(one, other sets.String) sets.String
- type Approval
- type ApprovedFile
- type Approvers
- func (ap *Approvers) AddApprover(login, reference string, noIssue bool)
- func (ap *Approvers) AddAssignees(logins ...string)
- func (ap *Approvers) AddAuthorSelfApprover(login, reference string, noIssue bool)
- func (ap *Approvers) AddLGTMer(login, reference string, noIssue bool)
- func (ap Approvers) AreFilesApproved() bool
- func (ap Approvers) GetCCs() []string
- func (ap Approvers) GetCurrentApproversSet() sets.String
- func (ap Approvers) GetCurrentApproversSetCased() sets.String
- func (ap Approvers) GetFiles(baseURL *url.URL, owner, repo, branch, providerType string) []File
- func (ap Approvers) GetFilesApprovers() map[string]sets.String
- func (ap Approvers) GetNoIssueApproversSet() sets.String
- func (ap Approvers) GetQuotedCCs(providerType string) []string
- func (ap Approvers) IsApproved() bool
- func (ap Approvers) ListApprovals() []Approval
- func (ap Approvers) ListNoIssueApprovals() []Approval
- func (ap Approvers) NoIssueApprovers() map[string]Approval
- func (ap *Approvers) RemoveApprover(login string)
- func (ap Approvers) RequirementsMet() bool
- func (ap Approvers) UnapprovedFiles() sets.String
- type File
- type Owners
- func (o Owners) GetAllPotentialApprovers() []string
- func (o Owners) GetApprovers() map[string]sets.String
- func (o Owners) GetLeafApprovers() map[string]sets.String
- func (o Owners) GetOwnersSet() sets.String
- func (o Owners) GetReverseMap(approvers map[string]sets.String) map[string]sets.String
- func (o Owners) GetShuffledApprovers() []string
- func (o Owners) GetSuggestedApprovers(reverseMap map[string]sets.String, potentialApprovers []string) sets.String
- func (o Owners) KeepCoveringApprovers(reverseMap map[string]sets.String, knownApprovers sets.String, ...) sets.String
- type Repo
- type UnapprovedFile
Constants ¶
const (
// ApprovalNotificationName defines the name used in the title for the approval notifications.
ApprovalNotificationName = "ApprovalNotifier"
)
Variables ¶
This section is empty.
Functions ¶
func GenerateTemplate ¶
GenerateTemplate takes a template, name and data, and generates the corresponding string.
func GetMessage ¶
func GetMessage(ap Approvers, linkURL *url.URL, org, repo, branch string, usePrefix bool, providerType string) *string
GetMessage returns the comment body that we want the approve plugin to display on PRs The comment shows:
- a list of approvers files (and links) needed to get the PR approved
- a list of approvers files with strikethroughs that already have an approver's approval
- a suggested list of people from each OWNERS files that can fully approve the PR
- how an approver can indicate their approval
- how an approver can cancel their approval
Types ¶
type Approval ¶
type Approval struct { Login string // Login of the approver (can include uppercase) How string // How did the approver approved Reference string // Where did the approver approved NoIssue bool // Approval also accepts missing associated issue }
Approval has the information about each approval on a PR
type ApprovedFile ¶
type ApprovedFile struct {
// contains filtered or unexported fields
}
ApprovedFile contains the information of a an approved file.
func (ApprovedFile) String ¶
func (a ApprovedFile) String() string
type Approvers ¶
type Approvers struct { AssociatedIssue int RequireIssue bool ManuallyApproved func() bool // contains filtered or unexported fields }
Approvers is struct that provide functionality with regard to approvals of a specific code change.
func NewApprovers ¶
NewApprovers create a new "Approvers" with no approval.
func (*Approvers) AddApprover ¶
AddApprover adds a new Approver
func (*Approvers) AddAssignees ¶
AddAssignees adds assignees to the list
func (*Approvers) AddAuthorSelfApprover ¶
AddAuthorSelfApprover adds the author self approval
func (Approvers) AreFilesApproved ¶
AreFilesApproved returns a bool indicating whether or not OWNERS files associated with the PR are approved. If this returns true, the PR may still not be fully approved depending on the associated issue requirement
func (Approvers) GetCCs ¶
GetCCs gets the list of suggested approvers for a pull-request. It now considers current assignees as potential approvers. Here is how it works: - We find suggested approvers from all potential approvers, but remove those that are not useful considering current approvers and assignees. This only uses leaf approvers to find the closest approvers to the changes. - We find a subset of suggested approvers from current approvers, suggested approvers and assignees, but we remove those that are not useful considering suggested approvers and current approvers. This uses the full approvers list, and will result in root approvers to be suggested when they are assigned. We return the union of the two sets: suggested and suggested assignees. The goal of this second step is to only keep the assignees that are the most useful.
func (Approvers) GetCurrentApproversSet ¶
GetCurrentApproversSet returns the set of approvers (login only, normalized to lower case)
func (Approvers) GetCurrentApproversSetCased ¶
GetCurrentApproversSetCased returns the set of approvers logins with the original cases.
func (Approvers) GetFilesApprovers ¶
GetFilesApprovers returns a map from files -> list of current approvers.
func (Approvers) GetNoIssueApproversSet ¶
GetNoIssueApproversSet returns the set of "no-issue" approvers (login only)
func (Approvers) GetQuotedCCs ¶ added in v0.0.632
GetQuotedCCs quotes ccs for @user usage if appropriate for the provider
func (Approvers) IsApproved ¶
IsApproved returns a bool indicating whether the PR is fully approved. If a human manually added the approved label, this returns true, ignoring normal approval rules.
func (Approvers) ListApprovals ¶
ListApprovals returns the list of approvals
func (Approvers) ListNoIssueApprovals ¶
ListNoIssueApprovals returns the list of "no-issue" approvals
func (Approvers) NoIssueApprovers ¶
NoIssueApprovers returns the list of people who have "no-issue" approved the pull-request. They are included in the list iff they can approve one of the files.
func (*Approvers) RemoveApprover ¶
RemoveApprover removes an approver from the list.
func (Approvers) RequirementsMet ¶
RequirementsMet returns a bool indicating whether the PR has met all approval requirements: - all OWNERS files associated with the PR have been approved AND EITHER
- the munger config is such that an issue is not required to be associated with the PR
- that there is an associated issue with the PR
- an OWNER has indicated that the PR is trivial enough that an issue need not be associated with the PR
func (Approvers) UnapprovedFiles ¶
UnapprovedFiles returns owners files that still need approval
type Owners ¶
type Owners struct {
// contains filtered or unexported fields
}
Owners provides functionality related to owners of a specific code change.
func NewOwners ¶
NewOwners consturcts a new Owners instance. filenames is the slice of files changed.
func (Owners) GetAllPotentialApprovers ¶
GetAllPotentialApprovers returns the people from relevant owners files needed to get the PR approved
func (Owners) GetApprovers ¶
GetApprovers returns a map from ownersFiles -> people that are approvers in them
func (Owners) GetLeafApprovers ¶
GetLeafApprovers returns a map from ownersFiles -> people that are approvers in them (only the leaf)
func (Owners) GetOwnersSet ¶
GetOwnersSet returns a set containing all the Owners files necessary to get the PR approved
func (Owners) GetReverseMap ¶
GetReverseMap returns a map from people -> OWNERS files for which they are an approver
func (Owners) GetShuffledApprovers ¶
GetShuffledApprovers shuffles the potential approvers so that we don't always suggest the same people.
func (Owners) GetSuggestedApprovers ¶
func (o Owners) GetSuggestedApprovers(reverseMap map[string]sets.String, potentialApprovers []string) sets.String
GetSuggestedApprovers solves the exact cover problem, finding an approver capable of approving every OWNERS file in the PR
func (Owners) KeepCoveringApprovers ¶
func (o Owners) KeepCoveringApprovers(reverseMap map[string]sets.String, knownApprovers sets.String, potentialApprovers []string) sets.String
KeepCoveringApprovers finds who we should keep as suggested approvers given a pre-selection knownApprovers must be a subset of potentialApprovers.
type Repo ¶
type Repo interface { Approvers(path string) sets.String LeafApprovers(path string) sets.String FindApproverOwnersForFile(file string) string IsNoParentOwners(path string) bool }
Repo allows querying and interacting with OWNERS information in a repo.
type UnapprovedFile ¶
type UnapprovedFile struct {
// contains filtered or unexported fields
}
UnapprovedFile contains the information of a an unapproved file.
func (UnapprovedFile) String ¶
func (ua UnapprovedFile) String() string