-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Configurable trailing slash behaviour for NormalizePath #1639
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good so far. Just left some initial thoughts and obviously the checks are failing from the doc comment in condition.rs.
Made TrailingSlash be non_exhaustive with a default of Always, and changed NormalizePath back to derive. Started storing the TrailingSlash enum in NormalizePathNormalization instead of a boolean. Added one more test request without a trailing slash. And also tidied up a bit.
Thanks for those updates. Add a CHANGELOG.md entry and a note to MIGRATION.md. Also check the job failure to see what needs updating. |
Codecov Report
@@ Coverage Diff @@
## master #1639 +/- ##
==========================================
+ Coverage 53.21% 53.22% +0.01%
==========================================
Files 124 126 +2
Lines 11760 11796 +36
==========================================
+ Hits 6258 6279 +21
- Misses 5502 5517 +15
Continue to review full report at Codecov.
|
Great work, thanks 👍 |
Hey, any plan to release it ? It's an awesome feature, I was able to have a |
PR Type
Feature
PR Checklist
Check your PR fulfills the following:
Overview
Currently NormalizePath always adds a trailing slash. This change would make it configurable to either follow that style, or to always trim the trailing slash instead.
Will be a breaking change to
NormalizePath
usage, sincemiddleware::NormalizePath
will need to be replaced bymiddleware::NormalizePath::default()
or::new(...)
when creating the middleware.Closes #1612