-
Notifications
You must be signed in to change notification settings - Fork 100
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
Add path field for RequestRedirect Filter #2979
Conversation
✅ Deploy Preview will be available once build job completes!
|
071a022
to
573d175
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2979 +/- ##
==========================================
+ Coverage 89.93% 89.95% +0.02%
==========================================
Files 111 111
Lines 11423 11452 +29
Branches 50 50
==========================================
+ Hits 10273 10302 +29
Misses 1089 1089
Partials 61 61 ☔ View full report in Codecov by Sentry. |
573d175
to
2c335e5
Compare
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
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.
Great work on this @salonichf5. Just a few suggestions.
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
site/content/how-to/traffic-management/redirects-and-rewrites.md
Outdated
Show resolved
Hide resolved
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.
🚀
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.
Let's try to get @ADubhlaoich's approval as well if possible
f9130e5
to
2d20bed
Compare
Co-authored-by: Saylor Berman <[email protected]>
2d20bed
to
2a614de
Compare
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.
LGTM!
Proposed changes
Write a clear and concise description that helps reviewers understand the purpose and impact of your changes. Use the
following format:
Problem: Users want to be able to provide path with the RequestRedirect filter.
Solution: Added functionality for path in RequestRedirect filter.
MainRewrite
increateMainRewriteForFilters
.Path
validator.Testing:
Added unit tests, conformance test and manual testing.
Configure the following HTTPRoutes for
tea
andsoda
applications following tutorialHTTPRoutes
Response for Request Redirect
/tea
Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.
Closes #1413
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.