-
Notifications
You must be signed in to change notification settings - Fork 527
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
implement delete manifest tests #915
Conversation
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.
lgtm in general, just a small style/consistency nit.
Thanks for this contribution! 🎉
Codecov Report
@@ Coverage Diff @@
## master #915 +/- ##
==========================================
- Coverage 72.67% 72.41% -0.27%
==========================================
Files 108 108
Lines 4696 4713 +17
==========================================
Hits 3413 3413
- Misses 773 789 +16
- Partials 510 511 +1
Continue to review full report at Codecov.
|
31c4b55
to
efe694e
Compare
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.
Couple nits, otherwise lgtm.
That would be wonderful, if you have the time! Thanks :) |
Signed-off-by: Carlos Panato <ctadeu@gmail.com>
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.
Thanks!
I was learning this repo to try to use in one personal thing and saw that some tests were pending to add.
This PR tries to implement the tests for
crane.Delete
, I'm not 100% if it is correct but it is a tryout 😄if sounds reasonable I will try to implement the other missing tests
/cc @jonjohnsonjr