uwiger / gproc

Extended process registry for Erlang
Apache License 2.0
1.07k stars 232 forks source link

warning with Erlang/OTP 20 #135

Open benoitc opened 7 years ago

benoitc commented 7 years ago

with latest version of Erlang OTP you get the following warnings:

_build/default/lib/gproc/src/gproc_pool.erl:379: Warning: crypto:rand_uniform/2 is deprecated and will be removed in a future release; use rand:uniform/1
_build/default/lib/gproc/src/gproc_pool.erl:605: Warning: crypto:rand_uniform/2 is deprecated and will be removed in a future release; use rand:uniform/1
_build/default/lib/gproc/src/gproc_pool.erl:1076: Warning: crypto:rand_uniform/2 is deprecated and will be removed in a future release; use rand:uniform/1
_build/default/lib/gproc/src/gproc_pool.erl:1088: Warning: crypto:rand_uniform/2 is deprecated and will be removed in a future release; use rand:uniform/1
_build/default/lib/gproc/src/gproc_pool.erl:1097: Warning: crypto:rand_uniform/2 is deprecated and will be removed in a future release; use rand:uniform/1

_build/default/lib/gproc/src/gproc_dist.erl:23: Warning: behaviour gen_leader undefined

I can fix them but not sure what is the appropriate way:

Let me know.

uwiger commented 7 years ago

Refactoring into a helper function seems like a good first step. Using function_exported/3 would probably work. The only thing to ensure is that an initial check is done to see if the module is loaded (e.g. calling code:ensure_loaded(rand) in the init function.)

tsloughter commented 7 years ago

Why not the compile macro that checks the version of OTP?

uwiger commented 7 years ago

Sorry, I now realize I was a little bit hasty before: my suggestion obviously doesn't address the deprecation warnings.

@tsloughter, specifically which compile macro did you have in mind?

A possibility would be to require OTP 18 or newer, and simply use rand:uniform/1. Would this be acceptable?

tsloughter commented 7 years ago

You can define a macro that does a regex on the otp version:

{platform_define, "^(19|2)", rand_only}

Though just requiring 18+ makes sense too now that 20 is out.

uwiger commented 7 years ago

The .travis.yml /really/ needs to be updated, since it actually only tests versions between R15B01 and 17.1, none of which arguably should be tested at all.

I'm fine with supporting only 18+ and later versions.