etalab / transport-validator

GTFS validator
https://transport.data.gouv.fr/validation/
MIT License
38 stars 10 forks source link

bump gtfs-structures version to 0.24.0 #111

Closed fchabouis closed 3 years ago

fchabouis commented 3 years ago

closes https://github.com/etalab/transport-site/issues/1660

There was a probleml with gtfs_structures::RawGtfs::from_url_async, making it impossible to fetch some remote resources.

See https://github.com/rust-transit/gtfs-structure/issues/78 for more details

fchabouis commented 3 years ago

@antoine-de or @Tristramg, my cargo.lock now contains dependencies versions. Versions were not specified before. Any idea why ? Thanks !

fchabouis commented 3 years ago

Also I had to add default values to a feed for a test to pass, I'm wondering why I had to do this, I haven't touched the code except for the version bump.

Tristramg commented 3 years ago

concerning the extra fields, it is because they were added by an external contribution https://github.com/rust-transit/gtfs-structure/commit/8ac9ce9b3688a3db24d6394c4b317f387f157720 We probably should implement Default to handle those extra fields without too much pain (but a default value for an Id does not make sense, so not as trivial)

For the explicit version, my guess (but not completly sure) is that gtfs-structure bumped to bytes 1 ( https://github.com/rust-transit/gtfs-structure/commit/d677624f4e3034701fbae4572ad2b3787769f959) while an other crate depends on bytes 0.5. Because there are two versions required, they must be defined explicitely

On Tue, 8 Jun 2021 at 09:46, Francis Chabouis @.***> wrote:

Also I had to add default values to a feed for a test to pass, I'm wondering why I had to do this, I haven't touched the code except for the version bump.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/etalab/transport-validator/pull/111#issuecomment-856540271, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAC7S6CNZJHVMX2YMDFLSLTRXDHFANCNFSM46HUVHUQ .

antoine-de commented 3 years ago

yep your actix version still uses bytes 0.5 so 2 versions needs to exist in your dependency tree (but that's ok)

you can update actix to "4.0.0-beta.6" to have bytes=1 (and tokio 1), but I'm not sure it's worth it.

I tried it a a branch, I can open a PR with it if you want since everything is ok with the bump (you can see it here I also tided up a dependency). I can also push in you branch if you think it's easier.

fchabouis commented 3 years ago

Thank you for the feedback guys. I will merge it now, but do not hesitate to open another PR for the proposed changes @antoine-de we will gladly accept it.