-
Notifications
You must be signed in to change notification settings - Fork 106
yasna & dependencies update #637
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
base: master
Are you sure you want to change the base?
Conversation
Taowyoo
left a comment
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 guess there are some code also need to change to sync up with upstream changes.
| url = "2.2" | ||
|
|
||
| mbedtls = { version = "0.12", features = ["std"], default-features = false, optional = true } | ||
| mbedtls = { version = "0.12", git = "https://github.com/fortanix/rust-mbedtls.git", branch = "mridul/yasna-0.5-crate-update", features = ["std"], default-features = false, optional = true } |
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.
Could you create an issue for tracking this, we should avoid using branch version?
|
Regarding CI failure, I guess there should be a |
|
|
||
| [dependencies] | ||
| b64-ct = "0.1.0" | ||
| b64-ct = "0.1.2" |
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.
Why this version constraint? With the specification b64-ct = "0.1.0" users are free to update to 0.1.2 if they choose. The same comment applies to many/most other changes in this PR.
See https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio
| em-node-agent-client = "1.0.0" | ||
| hyper = { version = "0.10", default-features = false } | ||
| mbedtls = { version = "0.12", default-features = false, features = ["rdrand", "std", "ssl"] } | ||
| mbedtls = { version = "0.12", git = "https://github.com/fortanix/rust-mbedtls.git", branch = "mridul/yasna-0.5-crate-update", default-features = false, features = ["rdrand", "std", "ssl"] } |
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.
Please don't specify dependencies by pointing to git repos. Does mbedtls even expose yasna types directly? If not, it could be updated to pick up yasna and be published as a patch update. Then users of em-app will pick yasna 0.5 as a dependency of mbedtls without any update
The currently used version of yasna is ver0.3, and it is advised to upgrade to latest yasna ver0.5.2.
This PR upgrades yasna version along with required dependencies.