Skip to content

Commit

Permalink
Respond to zmb3 feedback
Browse files Browse the repository at this point in the history
- Fix spelling.
- Add log if admin reviewers are assigned.
- Deduplicate preferred reviewers.
  • Loading branch information
ptgott committed Nov 16, 2023
1 parent 5390557 commit 860c394
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 2 deletions.
17 changes: 15 additions & 2 deletions bot/internal/review/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ func (r *Assignments) getDocsReviewers(e *env.Environment, files []github.PullRe

// If no docs reviewers were assigned, assign admin reviews.
if len(reviewers) == 0 {
log.Println("No docs reviewers found. Assigning admin reviewers.")
return r.getAdminReviewers(e.Author)
}
return reviewers
Expand Down Expand Up @@ -270,14 +271,26 @@ func (r *Assignments) getPreferredReviewers(set []string, files []github.PullReq

// getAllPreferredReviewers returns a list of reviewers that would be
// preferrable to review the provided changeset. Includes all preferred
// reviewers for each file path in the chagne set.
// reviewers for each file path in the changeset.
func (r *Assignments) getAllPreferredReviewers(set []string, files []github.PullRequestFile) (preferredReviewers []string) {
// Check each key in the preferred reviewer map, which is a file path
// that reviewers are assigned to. For any file names in the changeset
// that begin with that file path, add the reviewers for that pile path
// to the set of preferred reviewers. Look up each reviewer in a map to
// avoid duplication.
assigned := make(map[string]struct{})
for path, reviewers := range r.getPreferredReviewersMap(set) {
for _, file := range files {
if !strings.HasPrefix(file.Name, path) {
continue
}
preferredReviewers = append(preferredReviewers, reviewers...)
for _, rev := range reviewers {
if _, ok := assigned[rev]; ok {
continue
}
assigned[rev] = struct{}{}
preferredReviewers = append(preferredReviewers, rev)
}
}
}
return preferredReviewers
Expand Down
36 changes: 36 additions & 0 deletions bot/internal/review/review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,24 @@ func TestGetDocsReviewers(t *testing.T) {
},
reviewers: []string{"1", "2"},
},
{
desc: "preferred code reviewer for docs page with duplicate code reviewers",
assignments: &Assignments{
c: &Config{

CodeReviewers: map[string]Reviewer{
"2": {Team: "Core", Owner: true, PreferredReviewerFor: []string{"server-access", "database-access"}},
"3": {Team: "Core", Owner: true, PreferredReviewerFor: []string{"server-access", "database-access"}},
},
},
},
author: "4",
files: []github.PullRequestFile{
{Name: "server-access/get-started.mdx"},
{Name: "database-access/get-started.mdx"},
},
reviewers: []string{"2", "3"},
},
}
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
Expand Down Expand Up @@ -1012,6 +1030,24 @@ func TestCheckInternal(t *testing.T) {
large: false,
result: true,
},
{
desc: "docs-with-non-preferred-code-reviewer",
repository: "teleport",
author: "1",
reviews: []github.Review{
{Author: "3", State: Approved},
},
files: []github.PullRequestFile{
{
Name: "docs/pages/server-access/get-started.mdx",
},
},
docs: true,
code: false,
release: false,
large: false,
result: false,
},
}
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
Expand Down

0 comments on commit 860c394

Please sign in to comment.