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

🐛 fallback to local file detection on empty GitHub license response. #3412

Merged
merged 2 commits into from
Aug 23, 2023

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

bug fix

What is the current behavior?

If GitHub doesn't detect a license, the project gets a 0

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

We use our existing local file detection if github doesn't detect a license

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

Which issue(s) this PR fixes

Fixes #3279

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

Fixed bug where the Licenses folder wasn't being detected.

Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock spencerschrock temporarily deployed to gitlab August 22, 2023 18:52 — with GitHub Actions Inactive
@spencerschrock spencerschrock temporarily deployed to integration-test August 22, 2023 18:52 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #3412 (c9014b3) into main (eba10df) will decrease coverage by 6.32%.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3412      +/-   ##
==========================================
- Coverage   72.75%   66.44%   -6.32%     
==========================================
  Files         183      183              
  Lines       12938    12942       +4     
==========================================
- Hits         9413     8599     -814     
- Misses       3006     3859     +853     
+ Partials      519      484      -35     

Copy link
Contributor

@raghavkaul raghavkaul left a comment

Choose a reason for hiding this comment

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

Sounds good, but I'm not clear under which conditions the GitHub License API response would be empty and an actual license file could be found in the repo?

@spencerschrock
Copy link
Member Author

spencerschrock commented Aug 22, 2023

Sounds good, but I'm not clear under which conditions the GitHub License API response would be empty and an actual license file could be found in the repo?

#3279 is one such example. https://github.com/intel/ittapi

@spencerschrock spencerschrock temporarily deployed to gitlab August 23, 2023 18:00 — with GitHub Actions Inactive
@spencerschrock spencerschrock temporarily deployed to integration-test August 23, 2023 18:00 — with GitHub Actions Inactive
@spencerschrock spencerschrock merged commit f7409b3 into ossf:main Aug 23, 2023
37 of 38 checks passed
@spencerschrock spencerschrock deleted the fix/licenses-folder branch August 23, 2023 18:10
ashearin pushed a commit to kgangerlm/scorecard-gitlab that referenced this pull request Nov 13, 2023
…ssf#3412)

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Allen Shearin <allen.p.shearin@gmail.com>
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.

License check does not check License folder
2 participants