approvers

package
v0.0.5 Latest Latest
Warning

This package is not in the latest version of its module.

Go to latest
Published: Sep 7, 2018 License: Apache-2.0 Imports: 10 Imported by: 0

README

Reviewers and Approvers

Questions this Doc Seeks To Answer

  1. What do we need to get a PR merged into kubernetes/kubernetes?
  2. What are reviewers, approvers, and the OWNERS files?
  3. How does the reviewer selection mechanism work? approver selection mechanism work?
  4. 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 project
  • 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:

  1. Provide a subset of approvers that can approve all files in the PR

  2. Provide a small subset of approvers and suggest the same reviewers as blunderbuss if possible (people can be both reviewers and approvers)

  3. 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

Directory Structure

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**

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**

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**

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**

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.

Bot Notification for Approval Mechanism

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: mungegithub/mungers/blunderbuss.go

LGTM: prow/plugins/lgtm/lgtm.go

Approve: prow/plugins/approve/approve.go

mungegithub/mungers/approvers/owners.go

Documentation

Index

Constants

View Source
const (
	ApprovalNotificationName = "ApprovalNotifier"
)

Variables

This section is empty.

Functions

func GenerateTemplateOrFail

func GenerateTemplateOrFail(templ, name string, data interface{}) *string

GenerateTemplateOrFail takes a template, name and data, and generates the corresping string. nil is returned if it fails. An error is logged.

func IntersectSetsCase

func IntersectSetsCase(one, other sets.String) sets.String

IntersectSetsCase runs the intersection between to sets.String in a case-insensitive way. It returns the name with the case of "one".

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

func (Approval) String

func (a Approval) String() string

String creates a link for the approval. Use `Login` if you just want the name.

type ApprovedFile

type ApprovedFile struct {
	// contains filtered or unexported fields
}

func (ApprovedFile) String

func (a ApprovedFile) String() string

type Approvers

type Approvers struct {
	AssociatedIssue int
	RequireIssue    bool
	// contains filtered or unexported fields
}

func NewApprovers

func NewApprovers(owners Owners) Approvers

NewApprovers create a new "Approvers" with no approval.

func (*Approvers) AddApprover

func (ap *Approvers) AddApprover(login, reference string, noIssue bool)

AddApprover adds a new Approver

func (*Approvers) AddAssignees

func (ap *Approvers) AddAssignees(logins ...string)

AddAssignees adds assignees to the list

func (*Approvers) AddAuthorSelfApprover

func (ap *Approvers) AddAuthorSelfApprover(login, reference string)

AddSAuthorSelfApprover adds the author self approval

func (*Approvers) AddLGTMer

func (ap *Approvers) AddLGTMer(login, reference string, noIssue bool)

AddLGTMer adds a new LGTM Approver

func (Approvers) AreFilesApproved

func (ap Approvers) AreFilesApproved() bool

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

func (ap Approvers) GetCCs() []string

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 leave approvers to find approvers the closest to the changes. - We find a subset of suggested approvers from from current approvers, suggested approvers and assignees, but we remove thoses that are not useful considering suggestd 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

func (ap Approvers) GetCurrentApproversSet() sets.String

GetCurrentApproversSet returns the set of approvers (login only, normalized to lower case)

func (Approvers) GetCurrentApproversSetCased

func (ap Approvers) GetCurrentApproversSetCased() sets.String

GetCurrentApproversSetCased returns the set of approvers logins with the original cases.

func (Approvers) GetFiles

func (ap Approvers) GetFiles(org, project string) []File

UnapprovedFiles returns owners files that still need approval

func (Approvers) GetFilesApprovers

func (ap Approvers) GetFilesApprovers() map[string]sets.String

GetFilesApprovers returns a map from files -> list of current approvers.

func (Approvers) GetNoIssueApproversSet

func (ap Approvers) GetNoIssueApproversSet() sets.String

GetNoIssueApproversSet returns the set of "no-issue" approvers (login only)

func (Approvers) IsApproved

func (ap Approvers) IsApproved() bool

IsApproved returns a bool indicating whether the PR is fully approved: - 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) ListApprovals

func (ap Approvers) ListApprovals() []Approval

ListApprovals returns the list of approvals

func (Approvers) ListNoIssueApprovals

func (ap Approvers) ListNoIssueApprovals() []Approval

ListNoIssueApprovals returns the list of "no-issue" approvals

func (Approvers) NoIssueApprovers

func (ap Approvers) NoIssueApprovers() map[string]Approval

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

func (ap *Approvers) RemoveApprover(login string)

RemoveApprover removes an approver from the list.

func (Approvers) UnapprovedFiles

func (ap Approvers) UnapprovedFiles() sets.String

UnapprovedFiles returns owners files that still need approval

type File

type File interface {
	String() string
}

type Owners

type Owners struct {
	// contains filtered or unexported fields
}

func NewOwners

func NewOwners(filenames []string, r RepoInterface, s int64) Owners

func (Owners) GetAllPotentialApprovers

func (o Owners) GetAllPotentialApprovers() []string

GetAllPotentialApprovers returns the people from relevant owners files needed to get the PR approved

func (Owners) GetApprovers

func (o Owners) GetApprovers() map[string]sets.String

GetApprovers returns a map from ownersFiles -> people that are approvers in them

func (Owners) GetLeafApprovers

func (o Owners) GetLeafApprovers() map[string]sets.String

GetLeafApprovers returns a map from ownersFiles -> people that are approvers in them (only the leaf)

func (Owners) GetOwnersSet

func (o Owners) GetOwnersSet() sets.String

GetOwnersSet returns a set containing all the Owners files necessary to get the PR approved

func (Owners) GetReverseMap

func (o Owners) GetReverseMap(approvers map[string]sets.String) map[string]sets.String

GetReverseMap returns a map from people -> OWNERS files for which they are an approver

func (Owners) GetShuffledApprovers

func (o Owners) GetShuffledApprovers() []string

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 RepoInterface

type RepoInterface interface {
	Approvers(path string) sets.String
	LeafApprovers(path string) sets.String
	FindApproverOwnersForPath(path string) string
}

type UnapprovedFile

type UnapprovedFile struct {
	// contains filtered or unexported fields
}

func (UnapprovedFile) String

func (ua UnapprovedFile) String() string

Jump to

Keyboard shortcuts

? : This menu
/ : Search site
f or F : Jump to
y or Y : Canonical URL