Add option for requesting all possible reviewers (#130)

README.md

@@ -200,11 +200,17 @@ options:
 
   # Automatically request reviewers when a Pull Request is opened
   # if this rule is pending, there are no assigned reviewers, and if the
-  # Pull Request is not in Draft. Reviewers are selected randomly
-  # based on the set of requirements for this rule.
+  # Pull Request is not in Draft.
+  # Reviewers are selected based on the set of requirements for this rule
+  # and reviewers can be augmented using the mode option.
   request_review:
     # False by default
     enabled: true
+    # mode modifies how users are selected. `all-users` will request all users
+    # who are able to approve the pending rule. `random-users` selects a small
+    # set of random users based on the required count of approvals.
+    # defaults to 'random-users'
+    mode: all-users|random-users
 
   # "methods" defines how users may express approval. The defaults are below.
   methods:
@@ -371,17 +377,18 @@ limitations. Please file an issue if this functionality is important to you.
 `policy-bot` can automatically request reviewers for all pending rules
 when Pull Requests are opened by setting the `request_review` option.
 
+The `mode` enum modifies how users are selected. There are currently two
+supported options:
+ * `all-users` to request all users who can approve
+ * `random-users` to randomly select the number of users that are required
+
 ```yaml
 options:
   request_review:
-    # False by default
     enabled: true
+    mode: all-users|random-users
 ```
 
-A number of reviewers will be randomly requested based on the `requires` rules
-so that all rules will be satisfied once the set of requested reviewers complete
-and approve the Pull Request.
-
 The set of requested reviewers will not include the author of the Pull Request or
 users who are not collaborators on the repository.
 

godel/config/godel.properties

@@ -1,2 +1,2 @@
-distributionURL=https://palantir.bintray.com/releases/com/palantir/godel/godel/2.16.0/godel-2.16.0.tgz
-distributionSHA256=48b946ee2d55c64794e7b03eb64da6ff061f34a77350622f2bb6786932788d1a
+distributionURL=https://palantir.bintray.com/releases/com/palantir/godel/godel/2.18.0/godel-2.18.0.tgz
+distributionSHA256=b3bdbddb7e8347adbf9d3328b12efbf5d178e7dfc7353a2cacbbd12ef404b8f6

godelw

@@ -3,9 +3,9 @@
 set -euo pipefail
 
 # Version and checksums for godel. Values are populated by the godel "dist" task.
-VERSION=2.16.0
-DARWIN_CHECKSUM=71d997951823322fad1f87f6f634d5db0ab525c22ef5c7eb84091d822e6552a9
-LINUX_CHECKSUM=8bedeb60cc135304690799e6d375768bdc524d7d94a4bc5c1b9a89c86c5aa754
+VERSION=2.18.0
+DARWIN_CHECKSUM=fd748300ee17d3cb7ef348b0c76d800320974446fe447ae82a5273d64dbd9b54
+LINUX_CHECKSUM=de1159510634db0467e60bce67d38735498dd5f9be74bfb90d340621683cfb75
 
 # Downloads file at URL to destination path using wget or curl. Prints an error and exits if wget or curl is not present.
 function download {

policy/approval/approve.go

@@ -46,7 +46,8 @@ type Options struct {
 }
 
 type RequestReview struct {
-	Enabled bool `yaml:"enabled"`
+	Enabled bool               `yaml:"enabled"`
+	Mode    common.RequestMode `yaml:"mode"`
 }
 
 func (opts *Options) GetMethods() *common.Methods {
@@ -116,6 +117,8 @@ func (r *Rule) Evaluate(ctx context.Context, prctx pull.Context) (res common.Res
 				Admins:             r.Requires.Admins,
 				WriteCollaborators: r.Requires.WriteCollaborators,
 				RequiredCount:      r.Requires.Count,
+
+				Mode: r.Options.RequestReview.Mode,
 			}
 		}
 	}

policy/common/result.go

@@ -37,13 +37,22 @@ func (s EvaluationStatus) String() string {
 	return "unknown"
 }
 
+type RequestMode string
+
+const (
+	RequestModeAllUsers    RequestMode = "all-users"
+	RequestModeRandomUsers RequestMode = "random-users"
+)
+
 type ReviewRequestRule struct {
 	Teams              []string
 	Users              []string
 	Organizations      []string
 	Admins             bool
 	WriteCollaborators bool
 	RequiredCount      int
+
+	Mode RequestMode
 }
 
 type Result struct {
@@ -56,3 +65,11 @@ type Result struct {
 
 	Children []*Result
 }
+
+func (r Result) GetMode() RequestMode {
+	mode := RequestModeRandomUsers
+	if r.ReviewRequestRule.Mode != "" {
+		mode = r.ReviewRequestRule.Mode
+	}
+	return mode
+}

policy/reviewer/reviewer.go

@@ -125,7 +125,7 @@ func selectAdmins(prctx pull.Context) ([]string, error) {
 	return adminUsers, nil
 }
 
-func FindRandomRequesters(ctx context.Context, prctx pull.Context, result common.Result, r *rand.Rand) ([]string, error) {
+func SelectReviewers(ctx context.Context, prctx pull.Context, result common.Result, r *rand.Rand) ([]string, error) {
 	var usersToRequest []string
 	pendingLeafNodes := findLeafChildren(result)
 	zerolog.Ctx(ctx).Debug().Msgf("Found %d pending leaf nodes for review selection", len(pendingLeafNodes))
@@ -198,9 +198,17 @@ func FindRandomRequesters(ctx context.Context, prctx pull.Context, result common
 		}
 
 		if len(possibleReviewers) > 0 {
-			logger.Debug().Msgf("Found %d total candidates for review after removing author and non-collaborators; randomly selecting %d", len(possibleReviewers), child.ReviewRequestRule.RequiredCount)
-			randomSelection := selectRandomUsers(child.ReviewRequestRule.RequiredCount, possibleReviewers, r)
-			usersToRequest = append(usersToRequest, randomSelection...)
+			switch child.GetMode() {
+			case common.RequestModeAllUsers:
+				logger.Debug().Msgf("Found %d total reviewers after removing author and non-collaborators; requesting all", len(possibleReviewers))
+				usersToRequest = append(usersToRequest, possibleReviewers...)
+			case common.RequestModeRandomUsers:
+				logger.Debug().Msgf("Found %d total candidates for review after removing author and non-collaborators; randomly selecting %d", len(possibleReviewers), child.ReviewRequestRule.RequiredCount)
+				randomSelection := selectRandomUsers(child.ReviewRequestRule.RequiredCount, possibleReviewers, r)
+				usersToRequest = append(usersToRequest, randomSelection...)
+			default:
+				return nil, fmt.Errorf("unknown mode '%s' supplied", child.ReviewRequestRule.Mode)
+			}
 		} else {
 			logger.Debug().Msg("Did not find candidates for review after removing author and non-collaborators")
 		}

policy/reviewer/reviewer_test.go

@@ -69,7 +69,7 @@ func TestFindRepositoryCollaborators(t *testing.T) {
 	require.Equal(t, []string{"contributor-author", "contributor-committer", "mhaypenny", "org-owner", "review-approver", "user-direct-admin", "user-team-admin", "user-team-write"}, collabs)
 }
 
-func TestFindRandomRequesters(t *testing.T) {
+func TestSelectReviewers(t *testing.T) {
 	r := rand.New(rand.NewSource(42))
 	results := makeResults(&common.Result{
 		Name:        "Owner",
@@ -85,7 +85,7 @@ func TestFindRandomRequesters(t *testing.T) {
 
 	prctx := makeContext()
 
-	reviewers, err := FindRandomRequesters(context.Background(), prctx, results, r)
+	reviewers, err := SelectReviewers(context.Background(), prctx, results, r)
 	require.NoError(t, err)
 	require.Len(t, reviewers, 3, "policy should request three people")
 	require.Contains(t, reviewers, "review-approver", "at least review-approver must be selected")
@@ -94,7 +94,7 @@ func TestFindRandomRequesters(t *testing.T) {
 	require.NotContains(t, reviewers, "org-owner", "org-owner should not be requested")
 }
 
-func TestFindRandomRequesters_Team(t *testing.T) {
+func TestSelectReviewers_Team(t *testing.T) {
 	r := rand.New(rand.NewSource(42))
 	results := makeResults(&common.Result{
 		Name:        "Team",
@@ -110,7 +110,7 @@ func TestFindRandomRequesters_Team(t *testing.T) {
 	})
 
 	prctx := makeContext()
-	reviewers, err := FindRandomRequesters(context.Background(), prctx, results, r)
+	reviewers, err := SelectReviewers(context.Background(), prctx, results, r)
 	require.NoError(t, err)
 	require.Len(t, reviewers, 3, "policy should request three people")
 	require.Contains(t, reviewers, "review-approver", "at least review-approver must be selected")
@@ -119,7 +119,7 @@ func TestFindRandomRequesters_Team(t *testing.T) {
 	require.NotContains(t, reviewers, "not-a-collaborator", "a non collaborator cannot be requested")
 }
 
-func TestFindRandomRequesters_Org(t *testing.T) {
+func TestSelectReviewers_Org(t *testing.T) {
 	r := rand.New(rand.NewSource(42))
 	results := makeResults(&common.Result{
 		Name:        "Team",
@@ -135,7 +135,7 @@ func TestFindRandomRequesters_Org(t *testing.T) {
 	})
 
 	prctx := makeContext()
-	reviewers, err := FindRandomRequesters(context.Background(), prctx, results, r)
+	reviewers, err := SelectReviewers(context.Background(), prctx, results, r)
 	require.NoError(t, err)
 	require.Len(t, reviewers, 3, "policy should request three people")
 	require.Contains(t, reviewers, "review-approver", "at least review-approver must be selected")

server/handler/base.go

@@ -209,7 +209,7 @@ func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, pe
 
 		if !hasReviewers {
 			r := rand.New(rand.NewSource(time.Now().UnixNano()))
-			requestedUsers, err := reviewer.FindRandomRequesters(ctx, prctx, result, r)
+			requestedUsers, err := reviewer.SelectReviewers(ctx, prctx, result, r)
 			if err != nil {
 				return errors.Wrap(err, "Unable to select random request reviewers")
 			}