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

Jump to bottom

Fix routes command symbol usage #2366

Merged
merged 6 commits into from Jun 24, 2020
Merged

Fix routes command symbol usage #2366

merged 6 commits into from Jun 24, 2020

Conversation

stevapple
Copy link
Sponsor Contributor

@stevapple stevapple commented May 25, 2020 •
edited by tanner0101

Update routes command to keep consistency with symbols used by RoutingKit like *, :, etc (#2366).

@stevapple
Copy link
Sponsor Contributor Author

stevapple commented May 25, 2020 •
edited

Previously Vapor had its own implementation to parse the path, which would cause inconsistency in syntax with RoutingKit.

This patch uses description from RoutingKit, which outputs the exact syntax of path component (guaranteed by unit tests of RoutingKit). This helps Vapor to keep consistency no matter how the syntax changes.

@stevapple stevapple requested a review from tanner0101 May 25, 2020 04:25
@stevapple stevapple added semver-patch Internal changes only bug Something isn't working labels May 25, 2020
@tanner0101 tanner0101 added this to Awaiting Review in Vapor 4 via automation Jun 24, 2020
tanner0101
tanner0101 approved these changes Jun 24, 2020
View reviewed changes
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

Thanks!

@tanner0101 tanner0101 changed the title Update routes command to keep consistency Fix routes command symbol usage Jun 24, 2020
@tanner0101 tanner0101 merged commit 2d8a8c8 into vapor:master Jun 24, 2020
Vapor 4 automation moved this from Awaiting Review to Done Jun 24, 2020
@tanner0101
Copy link
Member

These changes are now available in 4.11.1

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

@tanner0101 tanner0101 tanner0101 approved these changes

Assignees
No one assigned
Labels
bug Something isn't working semver-patch Internal changes only
Projects
Vapor 4
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@stevapple @tanner0101