Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ cmd: Allow new scorecard to be instantiated with options #1703

Merged
merged 4 commits into from
Mar 3, 2022

Conversation

justaugustus
Copy link
Member

What kind of change does this PR introduce?

(Is it a bug fix, feature, docs update, something else?)
One part feature, one part bug fix?

  • PR title follows the guidelines defined in our pull request documentation

  • cmd: Allow new scorecard commands to be instantiated with options

  • options: Default flags to struct field values

  • options: Use constants for flag names

  • options: Simplify SARIF check

Signed-off-by: Stephen Augustus [email protected]

What is the current behavior?

With #1696, we enabled the ability to import the scorecard cobra command to support use cases like ossf/scorecard-action#122.

Unfortunately, because of the way we're currently handling defaulting for the flags, it means any initialization of the scorecard options gets discarded in favor of the flag defaults.

Where this comes up is when we attempt to build an options set via environment variables.

What is the new behavior (if this is a feature change)?**

  • (N/A) Tests for the changes have been added (for bug fixes/features)

First off, we allow the scorecard command to be initialized with options, which means a downstream consumer can now choose the way they build the options (ideally with o := options.New() + additional logic).

After that, we change how the flags default.
Instead of defaulting to empty values, we now use whatever value was initially set within Options.

BEFORE:

	cmd.Flags().StringVar(
		&o.Repo,
		FlagRepo,
		"repo",
		"repository to check",
	)

AFTER:

	cmd.Flags().StringVar(
		&o.Repo,
		FlagRepo,
		o.Repo,
		"repository to check",
	)

Which issue(s) this PR fixes

Special notes for your reviewer

h/t to @n3wscott for rubber-ducking with me!

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

- cmd: Allow new scorecard commands to be instantiated with options
- options: Default flags to struct field values
- options: Use constants for flag names
- options: Simplify SARIF check

@justaugustus justaugustus temporarily deployed to integration-test March 3, 2022 00:56 Inactive
@justaugustus justaugustus changed the title cmd: Allow new scorecard commands to be instantiated with options ✨ cmd: Allow new scorecard to be instantiated with options Mar 3, 2022
@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #1703 (ed88f45) into main (d192c8e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1703   +/-   ##
=======================================
  Coverage   57.91%   57.91%           
=======================================
  Files          58       58           
  Lines        5910     5910           
=======================================
  Hits         3423     3423           
  Misses       2248     2248           
  Partials      239      239           

@github-actions
Copy link

github-actions bot commented Mar 3, 2022

Integration tests success for
[6bbc2ec]
(https://github.com/ossf/scorecard/actions/runs/1925496558)

Copy link
Member

@naveensrinivasan naveensrinivasan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@justaugustus justaugustus enabled auto-merge (squash) March 3, 2022 01:32
@justaugustus justaugustus temporarily deployed to integration-test March 3, 2022 01:33 Inactive
@github-actions
Copy link

github-actions bot commented Mar 3, 2022

Integration tests success for
[ed88f45]
(https://github.com/ossf/scorecard/actions/runs/1925614141)

@justaugustus justaugustus merged commit 3070b3c into ossf:main Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants