-
Notifications
You must be signed in to change notification settings - Fork 86
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
feat: helper function in ioutil to PrintObjectAsTree #1129
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Horiodino <[email protected]>
@Horiodino Thanks for your contribution! Please fix the license and E2E test and add unit tests to cover the new code. |
@Horiodino You can check the CI running results to figure out the failure reasons. In general, you will need to add license header and E2E test to your code changes. |
No problem, almost done. I'll let you know once I push it. 👍 |
Signed-off-by: Horiodino <[email protected]>
I've identified and resolved an issue with the test cases. Initially, there wasn't a problem with the test cases themselves, but I had missed adding the |_ for some, which caused some tests to fail. I've fixed this, and now all test cases should pass. However, there is one test case that might still be failing: notation/test/e2e/suite/command/sign.go:297. I have cross-checked with the main repository and don't believe this failure is due to my recent commits, but please verify. |
all passed expect the one i mentioned above
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1129 +/- ##
==========================================
- Coverage 70.79% 70.25% -0.55%
==========================================
Files 48 48
Lines 2945 3026 +81
==========================================
+ Hits 2085 2126 +41
- Misses 668 701 +33
- Partials 192 199 +7 ☔ View full report in Codecov by Sentry. |
@Horiodino Sounds good! You have fixed the license and E2E. However, the test coverage doesn't reach 80%. You may need to add unit tests to improve the coverage. Thanks! |
@Horiodino Could you sign your commit by follow the guide. |
I already signed the commit |
go.mod
Outdated
@@ -28,6 +28,7 @@ require ( | |||
github.com/x448/float16 v0.8.4 // indirect | |||
golang.org/x/crypto v0.31.0 // indirect | |||
golang.org/x/mod v0.22.0 // indirect | |||
golang.org/x/sync v0.6.0 // indirect | |||
golang.org/x/net v0.33.0 // indirect |
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.
It looks like this PR doesn't need to update the go.mod.
You may need GPG key signing for the commit. Please have a look at the guide. |
Signed-off-by: Horiodino <[email protected]>
9f1e9d1
to
34b0e18
Compare
PR Summary
Fixes Issue: #545
Description of Changes:
PrintObjectAsTree
, in theioutil
package to improve functionality.tree
package from theinternal
package, as it is no longer needed.