Skip to content

Configurable handlers #108

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

Merged
merged 7 commits into from
Jun 19, 2019
Merged

Configurable handlers #108

merged 7 commits into from
Jun 19, 2019

Conversation

belak
Copy link
Collaborator

@belak belak commented Jun 12, 2019

This cleans up the configurable-handlers branch, brings some of our handler code more in line with what the stdlib does, and disables port forwarding by default with an option to enable it (we really shouldn't have shipped it like this by default).

This does break backwards compatibility (handler changes specifically) but adds quite a bit of flexibility without sacrificing complexity for people who just want to use this package out of the box.

This directly replaces #89

@bgardiner
Copy link

Thanks @belak. This looks great to me. I like exposing the ChannelHandlers and RequestHandlers maps publicly on the Server struct as opposed to the getters and setters in #89

@josegonzalez josegonzalez requested a review from progrium June 13, 2019 14:33
@gustavosbarreto
Copy link
Collaborator

Waiting for this pull request to be merged

@belak
Copy link
Collaborator Author

belak commented Jun 15, 2019

Hey all, I don't really have any projects to test this with any more - though that will be changing when I find the time. If you get a chance to check it and it works for you, let me know and I'll see about merging this.

@belak belak merged commit f199e8c into master Jun 19, 2019
@belak belak deleted the configurable-handlers branch June 19, 2019 07:27
@belak
Copy link
Collaborator Author

belak commented Jun 19, 2019

It's been a while and we've got people using similar setups already, so I feel comfortable merging this. Next tag will be a minor version bump (as with semver 0.x I'm pretty sure minor bumps can be breaking) rather than a patch version.

@bgardiner
Copy link

This is working great for me so far in a proxy server I wrote. Thanks for developing this and merging @belak

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.

5 participants