Skip to content
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

feat: Reduce severity of SameNameAndDescriptionForRouteNotice to WARNING #917

Merged
merged 2 commits into from
Jul 28, 2021

Conversation

aababilov
Copy link
Collaborator

A route that has the same route_desc and route_short_name or
route_long_name may be still served to the end user, so this is not an
error. In fact, many consumers may just ignore route_desc completely.

Numerous feeds that are consumed by Google do have this notice and they
are considered valid.

We also update the docs for the notice.

@aababilov aababilov requested a review from lionel-nj June 24, 2021 09:51
@isabelle-dr
Copy link
Contributor

isabelle-dr commented Jul 5, 2021

The specification is currently being reviewed to become RFC 2119 compliant (see changes here).
The ambiguity in routes.route_desc is planned on being solved by having:

Must not be a duplicate of route_short_name or route_long_name.

If this does not make a dataset invalid (and should trigger a WARNING in the validator), we should:

  1. Modify the change planned to the spec in PR227 to treat as a warning
  2. Modify the validator severity from error to warning to match the spec

@MobilityData/transit-specs thoughts?

@isabelle-dr isabelle-dr added the GTFS Reference Used for Adding or changing rules that belong in the GTFS reference label Jul 7, 2021
@aababilov
Copy link
Collaborator Author

I have posted a comment regarding route_desc on google/transit#277

I suggest to merge this pull request #917 first because the current behaviour of Validator treats plenty of feeds as invalid. This makes more difficult adoption of the Validator by members of GTFS community.

@isabelle-dr
Copy link
Contributor

Hello,

Thanks for your comment on google/transit!
I'm okay with merging this PR first @lionel-nj

Copy link
Contributor

@lionel-nj lionel-nj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on that @aababilov! One small documentation change should be provided here: SameNameAndDescriptionForRouteNotice should be in the table of warnings instead of the table or errors in RULES.md (same thing for the descriptions) and /docs/ NOTICES.md :)

A route that has the same route_desc and route_short_name or
route_long_name may be still served to the end user, so this is not an
error. In fact, many consumers may just ignore route_desc completely.

Numerous feeds that are consumed by Google do have this notice and they
are considered valid.

We also update the comment for the notice.
@aababilov
Copy link
Collaborator Author

Thanks for working on that @aababilov! One small documentation change should be provided here: SameNameAndDescriptionForRouteNotice should be in the table of warnings instead of the table or errors in RULES.md (same thing for the descriptions) and /docs/ NOTICES.md :)

Done. I think that would be great to develop a way for generating that doc from Java code.

@aababilov aababilov requested a review from lionel-nj July 28, 2021 01:15
Copy link
Contributor

@lionel-nj lionel-nj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aababilov!

I think that would be great to develop a way for generating that doc from Java code.

I agree, that would certainly help! Do you have examples of projects that have implemented this process in the past? Will definitely talk about this with @isabelle-dr and see how we can improve the gtfs-validator to this extent. Thanks for the suggestion!

@lionel-nj lionel-nj merged commit 3bdecfe into MobilityData:master Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GTFS Reference Used for Adding or changing rules that belong in the GTFS reference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants