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

:ssl.ssl_accept is deprecated in OTP 21 #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arcz
Copy link
Contributor

@arcz arcz commented Jul 3, 2018

No description provided.

@CrowdHailer
Copy link
Owner

This looks to break the tests.

Can you link me to that depreciation note for OTP21? and add a changelog fixed section for this

@arcz
Copy link
Contributor Author

arcz commented Jul 3, 2018

http://erlang.org/doc/man/ssl.html#ssl_accept-1

"Deprecated in OTP 21, use handshake/[1,2] instead."

The problem is that handshake is not present in OTP < 21, I'm not sure how this can be handled, should we wait until next release?

@CrowdHailer
Copy link
Owner

hmm. if we want ace to work with 20 and 21 we will need a compile time check on otp version. which is a pain. it's ashame there is not a period of overlap for the two functions

@CrowdHailer
Copy link
Owner

@arcz I've created a branch otp-21-update so see what would fail with the upgrade.

@CrowdHailer
Copy link
Owner

@CrowdHailer
Copy link
Owner

The problem of OTP 21 vs OTP 20 is going to be a non issue because I will support only OTP 21. we need to make this decision to use handle_continue.

@nietaki
Copy link
Contributor

nietaki commented Sep 19, 2018

I still don't think handle_continue is a big enough feature to justify dropping OTP 20 for...

@CrowdHailer
Copy link
Owner

agreed, but you get this warning just running ace on OTP21

and so this warning is actually preventing us from using warnings_as_errors: true in this project

@CrowdHailer
Copy link
Owner

I think we can probably resolve this now if we upgrade the versions of Elixir and OTP that Ace supports

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