Conversation
MarijnS95
left a comment
There was a problem hiding this comment.
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" |
|
|
||
| [dev-dependencies] | ||
| winit = "0.19.4" | ||
| winit = "0.26.1" |
There was a problem hiding this comment.
This probably needs additional changes, no?
There was a problem hiding this comment.
cargo check --workspace seemed to work
There was a problem hiding this comment.
--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.
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.
Again no need for patch versions.
There was a problem hiding this comment.
that's a bug in cargo upgrade --skip-compatible
| #![recursion_limit = "256"] | ||
|
|
||
| use heck::{CamelCase, ShoutySnakeCase, SnakeCase}; | ||
| use heck::{ToLowerCamelCase, ToShoutySnakeCase, ToSnakeCase}; |
There was a problem hiding this comment.
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 :(
| 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.
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.