-
Notifications
You must be signed in to change notification settings - Fork 110
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
move testutils under an ./internal/ directory #601
Conversation
@mikedanese - there's some things to fix so we can satisfy the CI but I agree it's better if these packages are internal 👍 Thanks for addressing this 🙏 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #601 +/- ##
=======================================
Coverage 70.18% 70.18%
=======================================
Files 10 10
Lines 2123 2123
=======================================
Hits 1490 1490
Misses 517 517
Partials 116 116
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Oops, fixed. |
FWIW, I was looking at using the RepositorySimulator in testutils as part of sigstore-go's test suite, but I can understand why it's probably best to keep it internal. |
@codysoyland - oh, I wasn't aware of that, thanks 👍 Given this I think it's okay to leave it as a standalone package. cc: @mikedanese edit: Of course, at the end we can always revisit and move it out of internal if someone comes up with a valid use case for it 👍 |
@rdimitrov Thanks for offering to keep it visible, but I decided late yesterday that it makes sense for us to have our own stripped down version of a repo simulator and not depend on the one here, since it may diverge and become incompatible, so I'm supportive of going ahead and merging this one! |
@codysoyland - Thanks! 😃👍 @mikedanese - There's a few conflicts you have to fix, but otherwise we should be good to go then 👍 |
This prevents these libraries from being depended on from outside the module. Signed-off-by: Mike Danese <[email protected]>
Rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The failing CI job is not related to the changes in this PR (tries to run go 1.22 and apparently something is not supporting it yet).
This prevents these libraries from being depended on from outside the module.