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

support multiple mutators #233

Merged
merged 3 commits into from
Aug 12, 2019

Conversation

jakkab
Copy link
Contributor

@jakkab jakkab commented Aug 8, 2019

Related issue

implements #233

Proposed changes

Support multiple mutators

Checklist

  • I have read the contributing guidelines
  • I have read the security policy
  • I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security
    vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation within the code base (if appropriate)
  • I have documented my changes in the developer guide (if appropriate)

@jakkab jakkab force-pushed the feature/support-multiple-mutators branch from a1acca6 to e458275 Compare August 8, 2019 09:45
@jakkab jakkab force-pushed the feature/support-multiple-mutators branch from e458275 to a624aa0 Compare August 8, 2019 09:48
@aeneasr
Copy link
Member

aeneasr commented Aug 8, 2019

😎

@aeneasr
Copy link
Member

aeneasr commented Aug 8, 2019

LMK when good for 👀

@jakkab
Copy link
Contributor Author

jakkab commented Aug 9, 2019

@aeneasr ready for review!

@jakkab jakkab marked this pull request as ready for review August 9, 2019 13:37
@jakkab jakkab force-pushed the feature/support-multiple-mutators branch from fe2d281 to a624aa0 Compare August 12, 2019 06:45
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

This looks super solid! Please:

  1. Add a section under "master" highlighting the change: https://github.com/ory/oathkeeper/blob/master/UPGRADE.md#master
  2. Update the docs ( https://github.com/ory/docs/tree/master/docs/oathkeeper ) from singular to plural!

Regarding the upgrade document, I think adding a pseudo-patch code section helps people the most. So something like this:

image

pipeline/mutate/mutator_cookie.go Show resolved Hide resolved
proxy/request_handler.go Outdated Show resolved Hide resolved
@jakkab
Copy link
Contributor Author

jakkab commented Aug 12, 2019

@aeneasr see ory/docs#193

@aeneasr
Copy link
Member

aeneasr commented Aug 12, 2019

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

@aeneasr aeneasr merged commit d21179d into ory:master Aug 12, 2019
@jakkab jakkab deleted the feature/support-multiple-mutators branch August 12, 2019 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants