-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add get_mut for fields of spec/runtime #166
Conversation
Fixes youki-dev#165 Signed-off-by: Alex Lyn <[email protected]>
Hi @utam0k @saschagrunert Could you please take a look this PR ? Thx |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
==========================================
- Coverage 22.42% 22.14% -0.28%
==========================================
Files 23 23
Lines 2100 2140 +40
Branches 1157 1167 +10
==========================================
+ Hits 471 474 +3
- Misses 809 843 +34
- Partials 820 823 +3 |
@Apokleos It seems CI got the failure. May I ask you to check it? |
This ToString trait is automatically implemented for any type which implements the Display trait. As such, ToString shouldn’t be implemented directly: Display should be implemented instead, and you get the ToString implementation for free. With such fixing, It will make CI with lint-clippy happy. Fixes youki-dev#165 Signed-off-by: Alex Lyn <[email protected]>
Hi @utam0k I need your help to execute the CI and help review the code. THX, FYI |
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, from my view. @saschagrunert PTAL 🙏
In cdi-go, the oci-runtime-tools generate tool is used to generate configuration JSON for the OCI runtime. However, since it is not possible to use existing libraries like cdi-go, we had to implement this functionality ourselves. Additionally, since the pr for oci-spec-rs (youki-dev/oci-spec-rs#166) has been merged but no release version exists yet, we'll utilize the below method for dependency management: oci-spec = { git = "https://github.com/containers/oci-spec-rs.git", rev = "d338cf8", features = ["runtime"] } Signed-off-by: Alex Lyn <[email protected]>
With oci-spec 0.6.6 [#166](youki-dev/oci-spec-rs#166) the `to_string()` method started to return the blanket impl of `ToString` due to the `Display` Trait being added. I'm not entirely sure why I got the display trait instead of the `to_string()` implemented on `ImageManifest`, in the rust playground I'm unable to reproduce it. Instead I now call serde_json directly so I don't have to rely on whatever oci-spec implements as the serialization.
With oci-spec 0.6.6 [#166](youki-dev/oci-spec-rs#166) the `to_string()` method started to return the blanket impl of `ToString` due to the `Display` Trait being added. I'm not entirely sure why I got the display trait instead of the `to_string()` implemented on `ImageManifest`, in the rust playground I'm unable to reproduce it. Instead I now call serde_json directly so I don't have to rely on whatever oci-spec implements as the serialization.
This layer collects and sends traces to the OTLP collector. Trace and Span IDs are now set on Span extensions instead of reusing the tracing generated IDs. fix: oci-spec update outputs pretty manifests With oci-spec 0.6.6 [#166](youki-dev/oci-spec-rs#166) the `to_string()` method started to return the blanket impl of `ToString` due to the `Display` Trait being added. I'm not entirely sure why I got the display trait instead of the `to_string()` implemented on `ImageManifest`, in the rust playground I'm unable to reproduce it. Instead I now call serde_json directly so I don't have to rely on whatever oci-spec implements as the serialization.
Fixes #165