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

docs: update #1009

Merged
merged 8 commits into from
Oct 12, 2021
Merged

docs: update #1009

merged 8 commits into from
Oct 12, 2021

Conversation

lionel-nj
Copy link
Contributor

@lionel-nj lionel-nj commented Sep 14, 2021

Summary:

This PR updates rules documentation. This includes changes to:

  • RULES.md
  • NOTICES.md
  • RouteNameValidator.java
  1. 0ab3485 addresses Clarify documentation of DuplicateRouteNameNotice and RouteShortAndLongNameEqualNotice #989
  2. 7557910 addresses Error in RULES.md for MissingLevelIdNotice #1008
  3. 4609ea7 addresses Add PathwayLoopValidator to the documentation #1001
  4. bb990a8 addresses Update documentation with ShapeToStopMatchingValidator rules #991
    Expected behavior:

No code change, same tests should pass.

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)

# Conflicts:
#	docs/NOTICES.md
@lionel-nj lionel-nj marked this pull request as ready for review October 7, 2021 16:36
Copy link
Contributor

@isabelle-dr isabelle-dr left a comment

Choose a reason for hiding this comment

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

Thank you @lionel-nj for working on improving the existing documentation and adding what was missing! I left a few comments :)

RULES.md Outdated
@@ -793,7 +803,7 @@ A route's color and `route_text_color` should be contrasting.

#### RouteShortAndLongNameEqualNotice

Short and long name are equal for a route.
Short and long name are equal for a single route.
Copy link
Contributor

Choose a reason for hiding this comment

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

I propose to do the following to clarify the difference between DuplicateRouteNameNotice and RouteShortAndLongNameEqualNotice using the tables @aababilov made in issue 971.

RouteShortAndLongNameEqualNotice

  1. description in the table at the top of the document:

route_short_name and route_long_name are equal for a single route

  1. longer description below

A single route has the same values for route_short_name and route_long_name.

route_id route_short_name route_long_name
route1 L1 L1

DuplicateRouteNameNotice

  1. description in the table at the top of the document:

Two routes have either the same route_short_name, the same route_long_name, or the same combination of route_short_name and route_long_name.

  1. longer description below
    Keep it as is but add:
route_id route_short_name route_long_name
route1 U1 Southern
route2 U1 Southern

RULES.md Show resolved Hide resolved
@lionel-nj
Copy link
Contributor Author

Thanks for review @isabelle-dr, PTAL.

@isabelle-dr
Copy link
Contributor

Thanks for the update @lionel-nj, three more things that might have not have been very clear in my comment:

  1. RouteShortAndLongNameEqualNotice short description in the table (README.md file):
    Short and long name are equal for a single route. -> route_short_name and route_long_name are equal for a single route.
  2. RouteShortAndLongNameEqualNotice longer description below (README.md file):
    Short and long name are equal for a single route. -> A single route has the same values for route_short_name and route_long_name.
  3. DuplicateRouteNameNotice short description in the table (README.md file):
    Duplicate routes.route_long_name. Duplicate routes.route_short_name. Duplicate combination of fields route_long_name and routes.route_short_name. -> Two distinct routes have either the same route_short_name, the same route_long_name, or the same combination of route_short_name and route_long_name.

@lionel-nj
Copy link
Contributor Author

My bad @isabelle-dr, just updated the PR with the modifications you suggested.

@isabelle-dr
Copy link
Contributor

LGTM! Thanks

@lionel-nj lionel-nj merged commit 8af5b39 into master Oct 12, 2021
@lionel-nj lionel-nj deleted the fix/rules-docs branch October 12, 2021 14:55
@lionel-nj
Copy link
Contributor Author

Thanks for reviewing @isabelle-dr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants