Don't look up approval candidates when no approval is required (#808)

We always check the approval candidates when evaluating an approval
policy. This process ends up making an API call to list the comments on
the PR, so that the approvers given in the policy can be checked against
the commenters or reviewers on the PR and only the relevant people
considered.

This search currently happens regardless of whether any approvals are
required. If there aren't any then we can save the API calls and a bit
of time by skipping the search. PRs can be evaluated fairly frequently
depending on the policy, so this can add up to a lot of API calls.

policy/approval/approve.go

@@ -304,6 +304,10 @@ func (r *Rule) isApprovedByConditions(ctx context.Context, prctx pull.Context) (
 // FilteredCandidates returns the potential approval candidates and any
 // candidates that should be dimissed due to rule options.
 func (r *Rule) FilteredCandidates(ctx context.Context, prctx pull.Context) ([]*common.Candidate, []*common.Dismissal, error) {
+	if r.Requires.Count <= 0 {
+		return nil, nil, nil
+	}
+
 	candidates, err := r.Options.GetMethods().Candidates(ctx, prctx)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "failed to get approval candidates")

policy/approval/approve_test.go

@@ -16,6 +16,7 @@ package approval
 
 import (
 	"context"
+	"errors"
 	"os"
 	"regexp"
 	"testing"
@@ -174,6 +175,11 @@ func TestIsApproved(t *testing.T) {
 
 	t.Run("noApprovalRequired", func(t *testing.T) {
 		prctx := basePullContext()
+		// There are no approvers required, so `Comments()` should not be
+		// called, and therefore this error should not be returned. We are
+		// checking that we don't make an unnecessary call to the GitHub API.
+		prctx.CommentsError = errors.New("Comments() was called")
+
 		r := &Rule{}
 		assertApproved(t, prctx, r, "No approval required")
 	})