Skip to content

Commit

Permalink
Add preferredOnly flag
Browse files Browse the repository at this point in the history
  • Loading branch information
jimbishopp committed Feb 7, 2024
1 parent c50647b commit e3ace4f
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
21 changes: 21 additions & 0 deletions bot/internal/review/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ func isAllowedRobot(author string) bool {
type Reviewer struct {
// Owner is true if the reviewer is a code or docs owner (required for all reviews).
Owner bool `json:"owner"`
// PreferredOnly is true if the reviewer should only be included in preferred reviewer paths.
PreferredOnly bool `json:"preferredOnly",omitempty`
// PreferredReviewerFor contains a list of file paths that this reviewer
// should be selected to review.
PreferredReviewerFor []string `json:"preferredReviewerFor,omitempty"`
Expand Down Expand Up @@ -286,10 +288,14 @@ func (r *Assignments) getCodeReviewers(e *env.Environment, files []github.PullRe
// pick from the overall set at random.
resultingSetA := preferredSetA
if len(resultingSetA) == 0 {
// Remove reviewers from setA whose preferredOnly field is false.
setA = filterPreferredOnly(reviewers, setA, false)
resultingSetA = append(resultingSetA, setA[r.c.Rand.Intn(len(setA))])
}
resultingSetB := preferredSetB
if len(resultingSetB) == 0 {
// Remove reviewers from setB whose preferredOnly field is false.
setB = filterPreferredOnly(reviewers, setB, false)
resultingSetB = append(resultingSetB, setB[r.c.Rand.Intn(len(setB))])
}

Expand Down Expand Up @@ -574,6 +580,21 @@ func reviewsByAuthor(reviews []github.Review) map[string]string {
return m
}

// filterPreferredOnly returns all names in set whose preferredOnly field is set to the preferredOnly parameter.
func filterPreferredOnly(reviewers map[string]Reviewer, set []string, preferredOnly bool) []string {
filtered := make([]string, 0, len(set))
for _, name := range set {
reviewer, ok := reviewers[name]
if !ok {
continue
}
if reviewer.PreferredOnly == preferredOnly {
filtered = append(filtered, name)
}
}
return filtered
}

const (
// Commented is a code review where the reviewer has left comments only.
Commented = "COMMENTED"
Expand Down
12 changes: 6 additions & 6 deletions bot/internal/review/review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func TestIsInternal(t *testing.T) {
}

// TestGetCodeReviewers checks internal code review assignments.
func TestGetCodeReviewers(t *testing.T) {
func TestGetCodeReviewerSets(t *testing.T) {
tests := []struct {
desc string
assignments *Assignments
Expand Down Expand Up @@ -1174,15 +1174,15 @@ func (r *randStatic) Intn(n int) int {
return 0
}

func TestPreferredReviewers(t *testing.T) {
func TestGetCodeReviewers(t *testing.T) {
assignments := &Assignments{
c: &Config{
Rand: &randStatic{},
CoreReviewers: map[string]Reviewer{
"1": {Owner: true, PreferredReviewerFor: []string{"lib/srv/db", "lib/srv/app"}},
"1": {Owner: true, PreferredReviewerFor: []string{"lib/srv/db", "lib/srv/app"}, PreferredOnly: true},
"2": {Owner: true, PreferredReviewerFor: []string{"lib/srv/db", "lib/alpn"}},
"3": {Owner: true},
"4": {Owner: false, PreferredReviewerFor: []string{"lib/srv/app"}},
"4": {Owner: false, PreferredReviewerFor: []string{"lib/srv/app"}, PreferredOnly: true},
"5": {Owner: false, PreferredReviewerFor: []string{"lib/srv/db"}},
"6": {Owner: false},
},
Expand All @@ -1209,7 +1209,7 @@ func TestPreferredReviewers(t *testing.T) {
files: []github.PullRequestFile{
{Name: "lib/alpn/proxy.go"},
},
expected: []string{"2", "4"},
expected: []string{"2", "5"},
},
{
description: "preferred reviewers for different files",
Expand All @@ -1226,7 +1226,7 @@ func TestPreferredReviewers(t *testing.T) {
files: []github.PullRequestFile{
{Name: "lib/service/service.go"},
},
expected: []string{"1", "4"},
expected: []string{"2", "5"},
},
{
description: "covered paths: don't add new or duplicate reviewers for paths already covered",
Expand Down

0 comments on commit e3ace4f

Please sign in to comment.