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

chore: check stop name and desc #937

Merged
merged 5 commits into from
Jul 26, 2021
Merged

Conversation

lionel-nj
Copy link
Contributor

closes #910

Summary:

This PR provides support to implement a new rule described in #910

Expected behavior:

New ValidationNotice: SameNameAndDescriptionForStopNotice (severity level: WARNING - to be upgraded to ERROR in the future.

A notice should be generated for each record from stops.txt where stops.stop_des == stops.stop_name.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • [ ] Include screenshot(s) showing how this pull request works and fixes the issue(s)

@lionel-nj lionel-nj self-assigned this Jul 23, 2021
@lionel-nj lionel-nj added the GTFS Reference Used for Adding or changing rules that belong in the GTFS reference label Jul 23, 2021
Copy link
Contributor

@maximearmstrong maximearmstrong left a comment

Choose a reason for hiding this comment

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

LGTM

@lionel-nj lionel-nj merged commit 78c291c into master Jul 26, 2021
@lionel-nj lionel-nj deleted the feat/check-stop-name-and-desc branch July 26, 2021 13:27
@isabelle-dr
Copy link
Contributor

This one can stay at the warning severity level, the spec after RFC says:

Should not be a duplicate of stop_name.

Copy link
Collaborator

@aababilov aababilov left a comment

Choose a reason for hiding this comment

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

Thanks! A couple of comments here.

|---------------- |----------------------------------------- |-------- |
| `csvRowNumber` | The row number of the faulty record. | String |
| `stopId` | The id of the faulty record. | Long |
| `routeDesc` | The faulty record's `stop_desc`. | String |
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: should be stopDesc

This is one more argument for removing this file. It is too easy to make a typo and it takes too much effort to catch all mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in PR #1004, opened issue #1006 to discuss the issues we have with NOTICES.md

@isabelle-dr isabelle-dr mentioned this pull request Oct 28, 2021
4 tasks
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.

Add validation on stops.stop_desc != stops.stop_name
4 participants