search

gen-smtp / gen_smtp

The extensible Erlang SMTP client and server library.
Other
683 stars 266 forks source link

Update Ranch dependency to >= 1.8.0 #285

Closed juhlig closed 3 years ago

juhlig commented 3 years ago

Ranch 1.7 is not compatible with OTP 24 because it uses the removed function ssl:ssl_accept. Ranch 1.8 was released to fix this.

seriyps commented 3 years ago

You may remove those lines as well:

https://github.com/gen-smtp/gen_smtp/blob/5ce941b38c22debb32b3b81ade21eb61bb1a4d96/.github/workflows/ci.yml#L21-L23

They were added exactly because 1.7 could not run on 1.8.

Also, remove this (and update CI accordingly, because with your PR tests run with test profile would run on the same ranch version as tests run on ranch18 profile):

https://github.com/gen-smtp/gen_smtp/blob/5ce941b38c22debb32b3b81ade21eb61bb1a4d96/rebar.config#L56-L57

Other than that, I think there were some reasons why we decided to hook to ranch 1.7, but I don't remember why exactly. I'll try to find some old conversations...

seriyps commented 3 years ago

Another point is that gen_smtp could work with ranch in a quite wide range of versions: from 1.7 to 2.0. It should be possible to specify in your top-level rebar.config that your project depends on ranch 1.8 and it will override gen_smtp's 1.7 dependency specifier without any issues (not sure if it works the same in Elixir/mix)

seriyps commented 3 years ago

Ok, after a bit of digging, it seems we fixed on 1.7 just because it was the most recent 1.x available by the time we bumped the version last time:

https://github.com/gen-smtp/gen_smtp/pull/218

So, I think we should we should bump to 1.8 indeed :+1: Just need to update CI and rebar.config the way I mentioned above

mworrell commented 3 years ago

I agree, we can do the CI things in a separate pull request.

Merging this, as it is about OTP-24 compatibility.