Redesigning the top-level Scorecard entrypoint

[spencerschrock]

Note: still tweaking this, but happy to get any feedback.

Problem

Our current top-level entrypoint, RunScorecard, has a long list of parameters.

func RunScorecard(ctx context.Context,
   repo clients.Repo,
   commitSHA string,
   commitDepth int,
   checksToRun checker.CheckNameToFnMap,
   repoClient clients.RepoClient,
   ossFuzzRepoClient clients.RepoClient,
   ciiClient clients.CIIBestPracticesClient,
   vulnsClient clients.VulnerabilitiesClient,
) (ScorecardResult, error)

There’s two problems with this:

1. Adding new options would require breaking changes to the function signature. While Scorecard has historically not cared about breaking changes, this is the single most important one, and we’ve broken it countless times when adding features.
2. Callers need to provide all parameters, even if they’re fine with the default values. Looking at our own usages, the vast majority of our calls use the default values.

Solution

We can use functional options to solve this problem. Functional options are a paradigm to pass an arbitrary number of options to a function to modify its behavior without changing its signature. At its core is the option type, which is unsurprisingly an alias for a function signature.

type Option func(*somePrivateState) error

The first step is splitting our current arguments into required and optional values. I’m arguing that the only required values are the context and the repo to analyze (for rationale, see Appendix). Every other existing argument will use default values, unless modified by an option:

func Run(ctx context.Context, repo clients.Repo, options ...Option) error {
    c := config{
        // some sensible defaults
    }
    for _, option := range options {
        if err := option(&c); err != nil {
            return err
        }
  }
  ... 

With this starting point, options are easily added. Let’s use commit SHA and depth as an example, we just add a new field to our unexported state, and an exported option to manipulate it:

type config struct {
    commit      string
    commitDepth int
}
func WithCommitDepth(depth int) Option {
    return func(c *config) error {
        c.commitDepth = depth
        return nil
    }
}
func WithCommitSHA(sha string) Option {
	return func(c *config) error {
		c.commit = sha
		return nil
	}
}

Existing callers don’t need to change their signature, but those that want to customize the behavior can:

Run(ctx, repo)
Run(ctx, repo, WithCommitDepth(42), WithCommitSHA("foo..."))

Appendix

A lot of the arguments either have default behavior built in already. Or the callers tend to provide the same arguments.

• commitSHA string,

  • clients.HeadSHA is the default, unless someone provides the --commit option

• commitDepth int

  • defaults to 30 if no option provided link

• checksToRun checker.CheckNameToFnMap

  • The default is to run all checks, but if --checks is provided, only those checks are run. However with structured results we're also seeing --probes added (see 🌱 Add probes to main call #3688)

• all of the repo clients

  • repoClient clients.RepoClient,
  • ossFuzzRepoClient clients.RepoClient,
  • ciiClient clients.CIIBestPracticesClient,
  • vulnsClient clients.VulnerabilitiesClient,
  • internal
    • internal usages tend to do one of two things:
      • use default clients. e.g. clients.DefaultCIIBestPracticesClient()
      • use checker.GetClients, which indirectly creates the default clients
  • external
    • use checker.GetClients

[github-actions]

This issue is stale because it has been open for 60 days with no activity.