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

Add option to provide custom WS config in zio-http #3291

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

kyri-petrou
Copy link
Contributor

Attempt at #3290.

@adamw I'm not sure what would be the best way to test these changes, do you have any suggestions? Also, the addition of the argument in ZioHttpServerOptions makes this a breaking change. Should we add the default value to the constructor of ZioHttpServerOptions to make it at least source compatible?

@adamw
Copy link
Member

adamw commented Oct 31, 2023

We don't have any compatibility requirements for this module (see https://tapir.softwaremill.com/en/latest/stability.html).

That's especially true for zio-http - every release (RC or not) usually means a major rewrite of the source code ;)

@adamw
Copy link
Member

adamw commented Oct 31, 2023

Which is to say: it's not a problem to add new options

@kyri-petrou
Copy link
Contributor Author

We don't have any compatibility requirements for this module (see https://tapir.softwaremill.com/en/latest/stability.html).

That's especially true for zio-http - every release (RC or not) usually means a major rewrite of the source code ;)

Ah nice, wasn't aware of this - that's good to know!

@kyri-petrou
Copy link
Contributor Author

@adamw tests for Scala 3 seem to be failing but I'm not entirely sure why. Any chance that's due to test flakyness?

@adamw adamw merged commit 21e13c4 into softwaremill:master Nov 2, 2023
@adamw
Copy link
Member

adamw commented Nov 2, 2023

Thanks :) Indeed, seems the test was flaky

@kyri-petrou kyri-petrou deleted the zio-http-custom-ws-config branch November 2, 2023 21:53
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.

2 participants