Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Code Ownership & Review Assignment Tool - GitHub CODEOWNERS but better

[![Go Report Card](https://goreportcard.com/badge/github.com/multimediallc/codeowners-plus)](https://goreportcard.com/report/github.com/multimediallc/codeowners-plus?kill_cache=1)
[![Tests](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml/badge.svg)](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml)
![Coverage](https://img.shields.io/badge/Coverage-81.6%25-brightgreen)
![Coverage](https://img.shields.io/badge/Coverage-81.7%25-brightgreen)
[![License](https://img.shields.io/badge/License-BSD%203--Clause-blue.svg)](https://opensource.org/licenses/BSD-3-Clause)
[![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-2.1-4baaaa.svg)](CODE_OF_CONDUCT.md)

Expand Down Expand Up @@ -225,6 +225,10 @@ ignore = ["test_project"]
# when one of these PR labels is present
high_priority_labels = ["high-priority", "urgent"]

# `detailed_reviewers` (default false) means the codeowners will include a collapsible list
# of files and owners in its review comment
detailed_reviewers = true

# `enforcement` allows you to specify how the Codeowners Plus check should be enforced
[enforcement]
# see "Enforcement Options" below for more details
Expand Down
2 changes: 2 additions & 0 deletions codeowners.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ unskippable_reviewers = ["@BakerNet"]
ignore = ["test_project"]
# `high_priority_lables` allows you to specify labels that should be considered high priority
high_priority_labels = ["P0"]
# `detailed_reviewers` (default false) means the codeowners will include a collapsible list of files and owners in its review comment
detailed_reviewers = false

# `enforcement` allows you to specify how the codeowners check should be enforced
[enforcement]
Expand Down
23 changes: 16 additions & 7 deletions internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,12 @@ func (a *App) addReviewStatusComment(allRequiredOwners codeowners.ReviewerGroups
comment += "\n\nThe PR has received the max number of required reviews. No further action is required."
}

if a.Conf.DetailedReviewers {
comment += "\n\n<details><summary>Show detailed file reviewers</summary>\n"
comment += a.getFileOwnersMapToString(a.codeowners.FileRequired())
comment += "</details>"
}

fiveDaysAgo := time.Now().AddDate(0, 0, -5)
existingComment, existingFound, err := a.client.FindExistingComment(commentPrefix, &fiveDaysAgo)
if err != nil {
Expand Down Expand Up @@ -447,14 +453,17 @@ func (a *App) requestReviews() error {
}

func (a *App) printFileOwners(codeOwners codeowners.CodeOwners) {
fileRequired := codeOwners.FileRequired()
a.printDebug("File Reviewers:\n")
for file, reviewers := range fileRequired {
a.printDebug("- %s: %+v\n", file, reviewers.Flatten())
}
fileOptional := codeOwners.FileOptional()
a.printDebug(a.getFileOwnersMapToString(codeOwners.FileRequired()))
a.printDebug("File Optional:\n")
for file, reviewers := range fileOptional {
a.printDebug("- %s: %+v\n", file, reviewers.Flatten())
a.printDebug(a.getFileOwnersMapToString(codeOwners.FileOptional()))
}

func (a *App) getFileOwnersMapToString(fileReviewers map[string]codeowners.ReviewerGroups) string {
builder := strings.Builder{}
for file, reviewers := range fileReviewers {
// builder.WriteString error return is always nil
_, _ = fmt.Fprintf(&builder, "- %s: %+v\n", file, reviewers.Flatten())
}
return builder.String()
}
64 changes: 64 additions & 0 deletions internal/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1178,3 +1178,67 @@ func TestBuildOutputData(t *testing.T) {
t.Errorf("expected StillRequired [@user1], got %v", output.StillRequired)
}
}

func TestCommentDetailedReviewers(t *testing.T) {
tt := []struct {
name string
detailedReviewers bool
}{
{
name: "exclude detailed owners when config off",
detailedReviewers: false,
},
{
name: "include detailed owners when config on",
detailedReviewers: true,
},
}

for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
mockGH := &mockGitHubClient{}

requiredOwners := codeowners.ReviewerGroups{
&codeowners.ReviewerGroup{Names: []string{"@user1", "@user2"}},
}
app := &App{
config: &Config{
InfoBuffer: io.Discard,
WarningBuffer: io.Discard,
},
client: mockGH,
codeowners: &mockCodeOwners{
fileRequiredMap: map[string]codeowners.ReviewerGroups{
"file1.go": {
&codeowners.ReviewerGroup{Names: []string{"@user1", "@user2"}},
},
"file2.go": {
&codeowners.ReviewerGroup{Names: []string{"@user1"}},
},
},
requiredOwners: requiredOwners,
},
Conf: &owners.Config{
HighPriorityLabels: []string{},
DetailedReviewers: tc.detailedReviewers,
},
}
err := app.addReviewStatusComment(requiredOwners, false)
if err != nil {
t.Errorf("unexpected error: %v", err)
}

DetailedReviewersSnippet := "\n\n<details><summary>Show detailed file reviewers</summary>\n" +
"- file1.go: [@user1 @user2]\n" +
"- file2.go: [@user1]\n"

containsDetailedReviewersSnippet := strings.Contains(mockGH.AddCommentInput, DetailedReviewersSnippet)

if tc.detailedReviewers != containsDetailedReviewersSnippet {
t.Errorf("expected comment to include detailed owners to be %t, got %t ",
tc.detailedReviewers, containsDetailedReviewersSnippet)
}

})
}
}
2 changes: 2 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type Config struct {
Enforcement *Enforcement `toml:"enforcement"`
HighPriorityLabels []string `toml:"high_priority_labels"`
AdminBypass *AdminBypass `toml:"admin_bypass"`
DetailedReviewers bool `toml:"detailed_reviewers"`
}

type Enforcement struct {
Expand All @@ -41,6 +42,7 @@ func ReadConfig(path string) (*Config, error) {
Enforcement: &Enforcement{Approval: false, FailCheck: true},
HighPriorityLabels: []string{},
AdminBypass: &AdminBypass{Enabled: false, AllowedUsers: []string{}},
DetailedReviewers: false,
}

fileName := path + "codeowners.toml"
Expand Down
3 changes: 3 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ ignore = ["ignored/"]
approval = true
fail_check = false
high_priority_labels = ["high-priority", "urgent"]
detailed_reviewers = true
`,
path: "testdata/",
expected: &Config{
Expand All @@ -46,6 +47,7 @@ high_priority_labels = ["high-priority", "urgent"]
Ignore: []string{"ignored/"},
Enforcement: &Enforcement{Approval: true, FailCheck: false},
HighPriorityLabels: []string{"high-priority", "urgent"},
DetailedReviewers: true,
},
expectedErr: false,
},
Expand All @@ -63,6 +65,7 @@ unskippable_reviewers = ["@user1"]
Ignore: []string{},
Enforcement: &Enforcement{Approval: false, FailCheck: true},
HighPriorityLabels: []string{},
DetailedReviewers: false,
},
expectedErr: false,
},
Expand Down