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

refactor OIDMatcher into separate utils package #454

Merged

Conversation

linus-sun
Copy link
Collaborator

Summary

This PR refactors the OIDMatcher type into a separate utils package, which will subsequently enable this PR to refactor the renderFulcioOIDMatchers function into a struct method (re: discussion here).

Release Note

NONE

Documentation

NONE- related documentation changes will happen in their respective PRs.

@linus-sun linus-sun force-pushed the linussun/update-identity-utils branch from f78bed1 to 7112812 Compare September 11, 2024 22:49
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.29%. Comparing base (d271ec7) to head (8504b2f).
Report is 100 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #454      +/-   ##
==========================================
- Coverage   64.02%   58.29%   -5.73%     
==========================================
  Files           4        7       +3     
  Lines         303      458     +155     
==========================================
+ Hits          194      267      +73     
- Misses         78      149      +71     
- Partials       31       42      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

This looks good, but question on pkg/util/file/file.go and pkg/util/file/file_test.go as they seem unrelated: do we want them in this PR?

@linus-sun
Copy link
Collaborator Author

This looks good, but question on pkg/util/file/file.go and pkg/util/file/file_test.go as they seem unrelated: do we want them in this PR?

I believe they're referenced because I moved them out of the pkg/util folder itself into a separate pkg/util/file folder so I can also put identity_utils in its own util subfolder, otherwise I'd have to put identity_utils in the file package, which doesn't seem suitable

@linus-sun
Copy link
Collaborator Author

linus-sun commented Sep 11, 2024

Just realized this duplicates the file utils, should be fixed

@linus-sun linus-sun force-pushed the linussun/update-identity-utils branch 2 times, most recently from 5f9aac5 to 3b73e49 Compare September 11, 2024 23:49
@linus-sun linus-sun marked this pull request as ready for review September 11, 2024 23:55
cmd/verifier/main.go Outdated Show resolved Hide resolved
pkg/util/identity/identity.go Outdated Show resolved Hide resolved
mihaimaruseac
mihaimaruseac previously approved these changes Sep 12, 2024
@haydentherapper haydentherapper merged commit 147878f into sigstore:main Sep 12, 2024
7 checks passed
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.

3 participants