test erlang 21

[elbrujohalcon]

Thanks @getong … I'll wait on Travis and if 🆗 … merge the PR

[elbrujohalcon]

Looks like we need to update rebar3 and remove calls to get_stacktrace.

[elbrujohalcon]

Hi @getong, thanks for the updates but we tend not to do things this way at @inaka. For once, we don't preserve backward compatibility this way. We also prefer not to use macros.

My recommendation for these changes would be to do one of these 2 options:

1. Just remove the calls to erlang:get_stacktrace(). That would leave the error logs with less info but fully backward compatible.
2. Just migrate to the new syntax. That would require us to specify on rebar.config and .travis.yml that we only support 21.0 from now on.

Approach 2 is what we usually do. Erlang developers working with older versions of OTP can just use older releases of worker_pool but in this case I wouldn't mind going with approach 1.

[getong]

@elbrujohalcon The syntax is right, but failed at ci. Need more time to fix it.

[elbrujohalcon]

@getong to be clear: I know the project should work with your changes, what I'm saying is that I won't accept such changes because they're against the philosophy of how we manage projects here.

In other words: I would not waste time trying to fix it if I were you. I would just implement one of the 2 solutions I proposed in my previous comment.

[getong]

Ok, I will make the erlang 21 as the default version, but the fail reason is not about the version. I need time to figure it out.

[michalwski]

Few cents from me, a heavy worker_pool user. I think removing erlang:get_stacktrace is a bad idea. This is often very useful information to debug why there was a crash. I also don't think that using new syntax and limit the Erlang version to 21 and newer is good. We at esl/MongooseIM heave the philosophy to support 3 latest Erlang version, now it's from 19 to 21 and we would still like to benefit from other changes made to worker_pool after this PR. And as you can see here: esl/MongooseIM#1947 both worker_pool and MongooseIM already work with Erlang 21.

Moreover the erlang:get_stacktrace is only deprecated in Erlang 21 which means it can still be used. Most probably it will still be there to use in 22.

[elbrujohalcon]

@michalwski: we can still move to 21 for our next release (say 3.2.0) and, after that one, keep applying back-patches to older versions if needed, producing releases like 3.1.1 and so on… This repo doesn't move so frequently for that to be an issue, I think. It's pretty stable and battle-tested code.

[getong]

@elbrujohalcon @michalslaski Finally, it works. if you want it only works with erlang 21, just add below to rebar.config

{minimum_otp_vsn, "21.0"}.

[getong]

@elbrujohalcon I will make a change tomorrow.

[getong]

@elbrujohalcon @michalwski
I rebase the branch to the master, and the build pass all the tests. Why the test needs to be fix spec, I still don’t know.

[getong]

@elbrujohalcon I reset the test code, and all tests are passed, it can be merged now.

[getong]

@elbrujohalcon would you make a new tag?

[michalwski]

Since now worker pool works only with Erlang/OTP 21 and newer, I strongly advise to create new release with the first number bumped to 4.I know the functionality didn't change but the compatibility with older version which are still popular and used was broken.

[elbrujohalcon]

Yeah… Look at #143. But I would like to implement #137 first.

[elbrujohalcon]

@getong you have https://hex.pm/packages/worker_pool/4.0.0 now, enjoy :)

[getong]

Thanks