:warning: Make all ScorecardResult format options pointers

ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source

https://scorecard.dev

Apache License 2.0

4.24k stars 460 forks source link

:warning: Make all ScorecardResult format options pointers #4151

Closed spencerschrock closed 2 weeks ago

spencerschrock commented 3 weeks ago

What kind of change does this PR introduce?

breaking change

[X] PR title follows the guidelines defined in our pull request documentation

What is the current behavior?

Some of the AsX functions take pointers (AsProbe), some take structs (AsString, AsJSON2)

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

All three AsX functions which have option structs, take pointers.

[ ] Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

One item from #4048

Special notes for your reviewer

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.)

NONE

spencerschrock commented 2 weeks ago

Just curious, what's the motivation to make all of the functions return pointers instead of all return values?

From my own experience, being able to pass nil is slightly shorter when I just want to use the defaults. But I took a look at what some of our other libraries do, and they both use pointers for options too, so I figured that was better.

For example the go-gitlab often uses pointers: https://pkg.go.dev/github.com/xanzy/go-gitlab#ProjectsService.GetProject As does our go-github library: https://pkg.go.dev/github.com/google/go-github/github#ChecksService.ListCheckRunsForRef

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 48.14815% with 14 lines in your changes missing coverage. Please review.

Project coverage is 60.37%. Comparing base (02f72e0) to head (e3d71e0). Report is 17 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #4151 +/- ## ========================================== - Coverage 66.11% 60.37% -5.75% ========================================== Files 232 212 -20 Lines 16567 15468 -1099 ========================================== - Hits 10954 9339 -1615 - Misses 4925 5433 +508 - Partials 688 696 +8 ```