search

etalab / transport-validator

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

Add a new rule about forbidding sub_folders #176

Closed antoine-de closed 10 months ago

antoine-de commented 10 months ago

closes #160

Add an error when data was in a subfolder.

175 and #174 needs to be merged first

antoine-de commented 10 months ago

@AntoineAugusti maybe some translations also needs to be done for this one.

If you want me to check those translations I can do it.

When all those PR will be merged, I'll also some chores to bump some dependencies.

thbar commented 10 months ago

Thanks!

I did a quick check to verify which GTFS may be impacted by this, because we have a global nightly import which raise errors on those cases.

In theory 10 files will be invalidated by this change.

81291 99100 error Unzip.Error File "stops.txt" not present in the zip 81185 104955 error Unzip.Error File "stops.txt" not present in the zip 81092 106888 error Unzip.Error File "stops.txt" not present in the zip 81090 107110 error Unzip.Error File "stops.txt" not present in the zip 81081 106903 error Unzip.Error File "stops.txt" not present in the zip 81091 106931 error Unzip.Error File "stops.txt" not present in the zip 81021 82763 error Unzip.Error File "stops.txt" not present in the zip 81199 92001 error Unzip.Error File "stops.txt" not present in the zip 81176 88220 error Unzip.Error File "stops.txt" not present in the zip

antoine-de commented 10 months ago

The first one even have a zip in the subfolder, also containing another GTFS :grinning:

AntoineAugusti commented 10 months ago

@antoine-de Could you share a sample JSON payload of the validator with this new rule? To craft great translation messages. https://github.com/etalab/transport-validator/pull/174#issuecomment-1803515440

antoine-de commented 10 months ago

Sure, I added a test with a json: https://github.com/etalab/transport-validator/pull/176/commits/60b929b8d1007f67307d82bdce5cfe8f70ad8825

Do you need more than this?

AntoineAugusti commented 10 months ago

@antoine-de Awesome, cheers! I was expecting a comment on GitHub but having a test is even better.

I'll craft a PR on the website side to implement a view/messages and link it here.

antoine-de commented 10 months ago

I rebased with the last merged PR.

In the meantime, I did a cargo update to update all dependencies and fix in the meantime #173

AntoineAugusti commented 10 months ago

@antoine-de Just opened a PR on the other repository to deal with this new error, thanks!

antoine-de commented 10 months ago

ok, we can merge this after the transport pr is merged, can you approve it when it will be done on your side?