-
Notifications
You must be signed in to change notification settings - Fork 684
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
implementation of add_request_headers and add_response_headers as obj… #1481
Conversation
@kflynn Can you tell me why the checks are failed, doesn't seems to be because of my changes, And please review the changes and let me know what all to done to have a merge. |
Code looks pretty reasonable, thanks! We already have tests in |
(You should also merge |
Merging from master
@kflynn merged from master to my fork, still having a similar error, |
Docs PR is also raised against master datawire/ambassador-docs#42 |
merging from master
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.
Hey, thanks for the great work 🎉
I have left some comments, one that asks to make the documentation a bit clearer, and another that asks for testing the behavior more than it currently does.
Also, I'm curious on how did you manually test this?
Co-Authored-By: ysaakpr <[email protected]>
merging from master
merging from master
Switch to watt 0.4.10.
Description
This change is to support the map based configuration for add_request_headers and add_response_headers, to support the advanced configuration of the 'append' flag in the virtual host config.
Related Issues
List related issues.
Testing
Haven't tested, only done linting
Todos
Other
master
.