-
Notifications
You must be signed in to change notification settings - Fork 346
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 to 2021 #405
Move to 2021 #405
Conversation
Codecov Report
@@ Coverage Diff @@
## main #405 +/- ##
==========================================
- Coverage 60.51% 60.47% -0.04%
==========================================
Files 82 82
Lines 12150 12148 -2
==========================================
- Hits 7353 7347 -6
- Misses 4797 4801 +4 |
2c02fe5
to
2d22ec6
Compare
.github/workflows/main.yml
Outdated
@@ -27,7 +27,7 @@ jobs: | |||
runs-on: ubuntu-latest | |||
strategy: | |||
matrix: | |||
rust: [1.55.0, 1.54.0] | |||
rust: [1.56.0] |
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.
I'd like to check the min and max versions in ci.
rust: [1.56.0] | |
rust: [1.54.0, 1.56.0] |
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.
Seems we must use 1.56.0 because of this: https://github.com/containers/youki/runs/3982169532?check_suite_focus=true#step:8:12
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.
Copy that. Can I ask you to indicate in the README that you need edition 2021.
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.
Copy that. Can I ask you to indicate in the README that you need edition 2021.
Done.
Btw, I got an warning:
--> crates/youki/src/main.rs:47:5
|
47 | log_format: Option<String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(dead_code)]` on by default
This field is not used?
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.
We have to take care of this option however we don't now. If we remove this code, we will surely get an error because it can not accept the option from the high-level container runtime.
If you have time to do it, why don't you try some research and revisions about this option?
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.
This is another issue, according to runc is used to set the format for logs.
--log-format value set the format used by logs ('text' (default), or 'json') (default: "text")
Just opened a issue for it: #423
I may have a try on it when have time.
Hey I think @utam0k has once previously noted that building with 2021 edition took quite a while, but I have tested with the |
Is it 1.56.1? It works fine for my local with rustc 1.56.1. |
Oops, yeah I meant 1.56.1 😅 😅 |
@chenyukang I believe this PR can restart! |
Synced with main, please review it. |
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.
🎉
closing #404