-
Notifications
You must be signed in to change notification settings - Fork 128
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
Upgrade Dependencies #212
Upgrade Dependencies #212
Conversation
.map(|f| f.ident.as_ref().unwrap()) | ||
.map(|ident| { | ||
use inflections::Inflect; | ||
let field = ident.as_ref().to_camel_case(); | ||
let field = ident.to_string().to_camel_case(); |
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 is to_string
needed?
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.
Using .as_ref() to get a str
was removed in syn 0.13, because proc_macro2 0.4 removed their Ident
AsStr
impl, as the normal proc_macro crate removed their AsStr
impl, the rationale for which is explained here.
Using .to_string() is now the only supported way to get the name of the identifier.
|
||
[dependencies.image] | ||
default-features = false | ||
features = ["jpeg", "png_codec"] |
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 better.
Thanks for your PR! The changes to the image crate are good. Adding The parallel JPEG decoding feature is OK but I would prefer it to be opt-in, that is, not enabled by default. |
This causes a minor break in backwards compatibility, due to images own break, involving the Format enum. Also, only require features necessary to support standard-required formats (jpg+png, see specification/2.0/schema/image.schema.json). A convenience feature to enable parallel JPEG decoding is also added.
Parallel JPEG decoding was only enabled by default because it had been in I also fixed the alphabetization on the third commit message. |
This PR updates the following outdated dependencies, and fixes the resulting incompatibilities:
approx
, 0.1.1 -> 0.3base64
, 0.6 -> 0.10cgmath
, 0.15 -> 0.17image
, 0.19 -> 0.21lazy_static
, 0.2 -> ^1quote
, 0.3 -> 0.6syn
, 0.11 -> 0.15Updating
quote
requires adding a dependency onproc_macro2
togltf-derive
, butgltf-json
already transitively requires that anyways.Updating
image
is slightly complicated by their adding of BGR pixel formats to theimage::DynamicImage
enum. The current PR just adds the new formats togltf::image::Format
, though you could, to preserve backwards compatibility, alternatively not add them and report some sort of error when encountered; or, to ensure forwards compatibility, add a hidden __Nonexhaustive variant (see RFC 2008 to prevent exhaustive matching.Additionally, this only asks for
image
features required by the glTF spec (that is, support for PNG and JPEG images), so as to decrease unused code size if someone is not usingimage
except throughgltf
.image
sjpeg_rayon
parallel JPEG decoding feature is also enabled by default for convenience. Alternatively, of course, we could not do this.