diff --git a/README.md b/README.md index 38efd9c..028f156 100644 --- a/README.md +++ b/README.md @@ -244,6 +244,10 @@ suppress_unowned_warning = true # If the PR author is included an OR group (e.g. `*.py @alice @bob`), the group is auto-satisfied without requiring another member to approve allow_self_approval = false +# `disable_review_status_comments` (default false) suppresses review status comments (required/unapproved reviewers). +# Optional reviewers are still invited with a CC comment. +disable_review_status_comments = false + # `enforcement` allows you to specify how the Codeowners Plus check should be enforced [enforcement] # see "Enforcement Options" below for more details diff --git a/internal/app/app.go b/internal/app/app.go index 8365e89..642019d 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -363,6 +363,10 @@ func (a *App) processApprovalsAndReviewers() (bool, string, []string, error) { func (a *App) addReviewStatusComment(allRequiredOwners codeowners.ReviewerGroups, maxReviewsMet bool, minReviewsNeeded int, currentApprovals int) error { // Comment on the PR with the codeowner teams required for review + if a.Conf.DisableReviewStatusComments { + a.printDebug("Skipping review status comment (explicitly disabled in config).\n") + return nil + } if a.config.Quiet { a.printDebug("Skipping review status comment (disabled or no unapproved owners).\n") return nil diff --git a/internal/app/app_test.go b/internal/app/app_test.go index f2788c0..0399572 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -529,16 +529,17 @@ func setupAppForTest(t *testing.T, quiet bool) (*App, *mockGitHubClient) { func TestAddReviewStatusComment(t *testing.T) { tt := []struct { - name string - requiredOwners codeowners.ReviewerGroups - maxReviewsMet bool - minReviewsNeeded int - currentApprovals int - existingComments []*github.IssueComment - expectAddComment bool - expectUpdateComment bool - expectError bool - expectMinReviewNote bool + name string + requiredOwners codeowners.ReviewerGroups + maxReviewsMet bool + minReviewsNeeded int + currentApprovals int + existingComments []*github.IssueComment + expectAddComment bool + expectUpdateComment bool + expectError bool + expectMinReviewNote bool + disableStatusComments bool }{ { name: "no existing comment", @@ -580,6 +581,16 @@ func TestAddReviewStatusComment(t *testing.T) { expectAddComment: true, expectMinReviewNote: true, }, + { + name: "min reviews not met but status comments disabled", + requiredOwners: codeowners.ReviewerGroups{ + &codeowners.ReviewerGroup{Names: codeowners.NewSlugs([]string{"@user1"})}, + }, + minReviewsNeeded: 3, + currentApprovals: 2, + disableStatusComments: true, + expectAddComment: false, + }, } for _, tc := range tt { @@ -598,7 +609,7 @@ func TestAddReviewStatusComment(t *testing.T) { codeowners: &mockCodeOwners{ requiredOwners: tc.requiredOwners, }, - Conf: &owners.Config{}, + Conf: &owners.Config{DisableReviewStatusComments: tc.disableStatusComments}, } err := app.addReviewStatusComment(tc.requiredOwners, tc.maxReviewsMet, tc.minReviewsNeeded, tc.currentApprovals) @@ -642,11 +653,12 @@ func TestAddOptionalCcComment(t *testing.T) { optionalMultiple := []string{"@cc-user1", "@cc-user2"} tt := []struct { - name string - quiet bool - optionalReviewers []string - expectedShouldCall bool - expectedComment string + name string + quiet bool + disableReviewStatusComments bool + optionalReviewers []string + expectedShouldCall bool + expectedComment string }{ { name: "short circuits in quiet mode", @@ -662,6 +674,14 @@ func TestAddOptionalCcComment(t *testing.T) { expectedShouldCall: true, expectedComment: fmt.Sprintf("cc %s", strings.Join(optionalMultiple, " ")), }, + { + name: "adds a cc comment even if review status comments are disabled", + quiet: false, + disableReviewStatusComments: true, + optionalReviewers: optionalMultiple, + expectedShouldCall: true, + expectedComment: fmt.Sprintf("cc %s", strings.Join(optionalMultiple, " ")), + }, } for _, tc := range tt { @@ -669,6 +689,8 @@ func TestAddOptionalCcComment(t *testing.T) { app, mockClient := setupAppForTest(t, tc.quiet) mockClient.ResetGHClientTracking() + app.Conf.DisableReviewStatusComments = tc.disableReviewStatusComments + err := app.addOptionalCcComment(tc.optionalReviewers) if err != nil { t.Errorf("Unexpected error when adding optional cc comment: %v", err) diff --git a/internal/config/config.go b/internal/config/config.go index 537a33b..fd1b6e8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,18 +8,19 @@ import ( ) type Config struct { - MaxReviews *int `toml:"max_reviews"` - MinReviews *int `toml:"min_reviews"` - UnskippableReviewers []string `toml:"unskippable_reviewers"` - Ignore []string `toml:"ignore"` - Enforcement *Enforcement `toml:"enforcement"` - HighPriorityLabels []string `toml:"high_priority_labels"` - AdminBypass *AdminBypass `toml:"admin_bypass"` - DetailedReviewers bool `toml:"detailed_reviewers"` - DisableSmartDismissal bool `toml:"disable_smart_dismissal"` - RequireBothBranchReviewers bool `toml:"require_both_branch_reviewers"` - SuppressUnownedWarning bool `toml:"suppress_unowned_warning"` - AllowSelfApproval bool `toml:"allow_self_approval"` + MaxReviews *int `toml:"max_reviews"` + MinReviews *int `toml:"min_reviews"` + UnskippableReviewers []string `toml:"unskippable_reviewers"` + Ignore []string `toml:"ignore"` + Enforcement *Enforcement `toml:"enforcement"` + HighPriorityLabels []string `toml:"high_priority_labels"` + AdminBypass *AdminBypass `toml:"admin_bypass"` + DetailedReviewers bool `toml:"detailed_reviewers"` + DisableSmartDismissal bool `toml:"disable_smart_dismissal"` + RequireBothBranchReviewers bool `toml:"require_both_branch_reviewers"` + SuppressUnownedWarning bool `toml:"suppress_unowned_warning"` + AllowSelfApproval bool `toml:"allow_self_approval"` + DisableReviewStatusComments bool `toml:"disable_review_status_comments"` } type Enforcement struct { @@ -38,16 +39,17 @@ func ReadConfig(path string, fileReader codeowners.FileReader) (*Config, error) } defaultConfig := &Config{ - MaxReviews: nil, - MinReviews: nil, - UnskippableReviewers: []string{}, - Ignore: []string{}, - Enforcement: &Enforcement{Approval: false, FailCheck: true}, - HighPriorityLabels: []string{}, - AdminBypass: &AdminBypass{Enabled: false, AllowedUsers: []string{}}, - DetailedReviewers: false, - DisableSmartDismissal: false, - RequireBothBranchReviewers: false, + MaxReviews: nil, + MinReviews: nil, + UnskippableReviewers: []string{}, + Ignore: []string{}, + Enforcement: &Enforcement{Approval: false, FailCheck: true}, + HighPriorityLabels: []string{}, + AdminBypass: &AdminBypass{Enabled: false, AllowedUsers: []string{}}, + DetailedReviewers: false, + DisableSmartDismissal: false, + RequireBothBranchReviewers: false, + DisableReviewStatusComments: false, } // Use filesystem reader if none provided diff --git a/internal/config/config_test.go b/internal/config/config_test.go index cbc26db..f09c117 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -38,16 +38,18 @@ approval = true fail_check = false high_priority_labels = ["high-priority", "urgent"] detailed_reviewers = true +disable_review_status_comments = true `, path: "testdata/", expected: &Config{ - MaxReviews: intPtr(2), - MinReviews: intPtr(1), - UnskippableReviewers: []string{"@user1", "@user2"}, - Ignore: []string{"ignored/"}, - Enforcement: &Enforcement{Approval: true, FailCheck: false}, - HighPriorityLabels: []string{"high-priority", "urgent"}, - DetailedReviewers: true, + MaxReviews: intPtr(2), + MinReviews: intPtr(1), + UnskippableReviewers: []string{"@user1", "@user2"}, + Ignore: []string{"ignored/"}, + Enforcement: &Enforcement{Approval: true, FailCheck: false}, + HighPriorityLabels: []string{"high-priority", "urgent"}, + DetailedReviewers: true, + DisableReviewStatusComments: true, }, expectedErr: false, },