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.

Sign up for GitHub

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

Jump to bottom

fix: validate route, stops and agency consistency urls #1003

Merged
merged 5 commits into from
Sep 9, 2021
Merged

fix: validate route, stops and agency consistency urls #1003

merged 5 commits into from
Sep 9, 2021

Conversation

lionel-nj
Copy link
Contributor

relates to #907 and #909

Summary:

This PR reworks the rule logic implemented in #932:

Expected behavior:

  • a notice is generated for each stop/agency/route whose url is duplicate.

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 Sep 8, 2021
@lionel-nj lionel-nj mentioned this pull request Sep 8, 2021
Merged
6 tasks
aababilov
aababilov requested changes Sep 8, 2021
View reviewed changes
Copy link
Collaborator

@aababilov aababilov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! Very nice, thanks a lot! Some minor comments regarding the style.

main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Outdated Show resolved Hide resolved
apply suggestions from code review
4b8eb25 
- fix typos
- simplify code
- update docs
- remove unused method
aababilov
aababilov approved these changes Sep 8, 2021
View reviewed changes
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!

docs/NOTICES.md Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Outdated Show resolved Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/validator/UrlConsistencyValidator.java Show resolved Hide resolved
@lionel-nj lionel-nj merged commit afc24e0 into master Sep 9, 2021
@lionel-nj lionel-nj deleted the fix/url-consistency-validator branch September 9, 2021 16:52
@lionel-nj
Copy link
Contributor Author

Thanks for review @aababilov.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aababilov aababilov Awaiting requested review from aababilov

Assignees

@lionel-nj lionel-nj

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lionel-nj @aababilov