-
Notifications
You must be signed in to change notification settings - Fork 186
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
Update dependencies. #547
Update dependencies. #547
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.
The ash-window change should be reverted as we need to give that some more time and thought. Then, this only bumps example and generator dependencies which at least have no effect on the released crates.
raw-window-handle = "0.3" | ||
raw-window-handle = "0.4.2" | ||
|
||
[target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies] | ||
raw-window-metal = "0.1" | ||
raw-window-metal = "0.2.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.
|
||
[dev-dependencies] | ||
winit = "0.19.4" | ||
winit = "0.26.1" |
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 probably needs additional changes, no?
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.
cargo check --workspace seemed to work
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.
--workspace
only checks the crates (binaries, libraries) in the workspace
(and it seems to be the default when the root Cargo.toml
only defines the workspace but doesn't contain any crate of its own).
You should however also check
the examples with --examples
, or more generically --all-targets
.
winit = "0.25.0" | ||
image = "0.10.4" | ||
winit = "0.26.1" | ||
image = "0.23.14" |
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.
Do we need these minimum patch versions? Otherwise just omit them entirely, this is just an example.
itertools = "0.10" | ||
nom = "6.0" | ||
once_cell = "1.7" | ||
once_cell = "1.9.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.
Again no need for patch versions.
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.
that's a bug in cargo upgrade --skip-compatible
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 do these by hand :)
@@ -1,6 +1,6 @@ | |||
#![recursion_limit = "256"] | |||
|
|||
use heck::{CamelCase, ShoutySnakeCase, SnakeCase}; | |||
use heck::{ToLowerCamelCase, ToShoutySnakeCase, ToSnakeCase}; |
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.
Can we use the cleaner As*
wrappers in formatting places?
EDIT: We can't in the places of format_ident!
which calls their own limited IdentFragment
trait instead of Display::fmt
:(
@@ -1200,7 +1200,7 @@ pub fn generate_extension_commands<'a>( | |||
|
|||
let ident = format_ident!( | |||
"{}Fn", | |||
extension_name.to_camel_case().strip_prefix("Vk").unwrap() | |||
extension_name.to_lower_camel_case().strip_prefix("Vk").unwrap() |
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.
Lower... So this won't strip the uppercase Vk prefix if the whole thing is lowered or will it retain characters that were uppercase?
Please run the generator at least once to make sure output doesn't change or remains what we expect.
Closing in favor of #505, sorry didn't see it sooner. |
No need to insta-close, we could use the rest of the upgrades but they need some more thought. Whenever upgrading libraries I always do a quick check in the changelog to see if there's anything useful, new features, or worrying shortcomings that I should keep in mind (which my only show at runtime). |
No description provided.