gltf-rs / gltf

A crate for loading glTF 2.0
Apache License 2.0
534 stars 124 forks source link

Consider transitioning `gltf-derive` to use `syn@2` instead of `syn@1` #372

Closed LikeLakers2 closed 1 year ago

LikeLakers2 commented 1 year ago

Hi! I noticed that gltf-derive (and possibly other parts of this repository) depend on syn@1. Perhaps you could consider updating to use syn@2 wherever possible?

Some things worth noting for anyone who intends to take on this suggestion:

If you do consider to transition to syn@2, you have my thanks! :)

Elabajaba commented 1 year ago

syn 2 is as easy as just changing it from 1 to 2 in cargo.toml, and everything still seems to work in my limited testing.

The current stated msrv is wrong because image 0.24.0 has an msrv of 1.56, and image 0.24.6 has an msrv of 1.61.

The hard part is that if we're raising the msrv that high, we might as well switch to the 2021 edition, but that breaks compilation. ("trait objects must include the dyn keyword" error on the Validate macro from gltf-derive everywhere it's used in gltf-json, see https://github.com/gltf-rs/gltf/issues/355)

edit: Regarding the msrv, image seems to bump it relatively often in patch releases for performance wins so I'm not sure if gltf should even bother with an msrv since it isn't very actively maintained, and it'd be getting version bumps relatively often.