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

🌱 Fix pinsDependencies outcomes #3961

Merged
merged 6 commits into from
Mar 29, 2024

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

probe cleanup

What is the current behavior?

  • OutcomeNotAvailable was used for cases where there are no dependencies.
    • The description mentions this is for API issues which isn't the case here.
  • OutcomeNotApplicable was used for cases where our data is inconsistent?
  • Probe outcome descriptions are wrong

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

  • Inconsistent data states are now OutcomeNotSupported, as OutcomeError is already used for processing errors (🐛 Pinned-Dependencies continues on error #3515)
    • I'm not sure this is a perfect match, open to suggestions
  • Situations with no supported dependencies now use OutcomeNotApplicable
  • Probe documentation updated to fix copy/paste mistake
  • Tests were added for processing errors and no dependencies
  • Finding locations were simplified (see 6439b05 for details)
  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #3855

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

OutcomeNotApplicable is what we've been using for cases where there are no occurences of X.
Previously this outcome was used for this probe to handle some error cases, but
OutcomeError is currently being used. Existing callers were moved to OutcomeNotSupported.

Signed-off-by: Spencer Schrock <sschrock@google.com>
checker.File.Location() is nil safe, so this should work when we have a location or not

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Merging #3961 (ea7c906) into main (2a45ba6) will decrease coverage by 6.40%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3961      +/-   ##
==========================================
- Coverage   74.92%   68.53%   -6.40%     
==========================================
  Files         223      223              
  Lines       16086    16046      -40     
==========================================
- Hits        12053    10997    -1056     
- Misses       3264     4348    +1084     
+ Partials      769      701      -68     

@spencerschrock
Copy link
Member Author

/scdiff generate Pinned-Dependencies

Copy link

@spencerschrock spencerschrock marked this pull request as ready for review March 20, 2024 18:38
@spencerschrock spencerschrock requested a review from a team as a code owner March 20, 2024 18:38
@spencerschrock spencerschrock requested review from justaugustus and raghavkaul and removed request for a team March 20, 2024 18:38
probes/pinsDependencies/impl.go Show resolved Hide resolved
@spencerschrock spencerschrock enabled auto-merge (squash) March 29, 2024 21:50
@spencerschrock spencerschrock merged commit 46eea0e into ossf:main Mar 29, 2024
38 checks passed
@spencerschrock spencerschrock deleted the pinned-deps-probe-cleanup branch March 29, 2024 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Revisit outcomes in Pinned Dependencies probes
2 participants