search

josefs / Gradualizer

A Gradual type system for Erlang
MIT License
613 stars 35 forks source link

`binary_to_atom/1` breaks compatibility below OTP 23 #552

Open robertfiko opened 1 year ago

robertfiko commented 1 year ago

In the Readme, it is stated, that Erlang/OTP 21+ is required, but the binary_to_atom/1 was introduced in OTP 23. I suggest using, the binary_to_atom/2 with the second parameter, as utf8, which is basically how it is done in OTP 23.

https://www.erlang.org/doc/man/erlang.html#binary_to_atom-1

I can do the fix ofc., but wanted to consult with a maintainer.

zuiderkwast commented 1 year ago

It would be good to have a CI job running on OTP 21.

erszcz commented 1 year ago

I think the README should be updated to reflect the current CI. IMO maintenance effort this many versions backwards will be a problem and we've not even caught up with OTP 25, not to mention 26.

That being said, if a simple fix is sufficient in this case, I think a PR would be swiftly merged :)

robertfiko commented 1 year ago

If that's the only part where compatibility breaks, I would prefer to do this fix, and I would really appreciate compatibility with at least OTP 22.

I will prepare a PR soon.

zuiderkwast commented 1 year ago

Great. If we want a github CI job for OTP 22, we'd need to install an old rebar3 which supports it. We can copy this rule from the eredis project:

      - name: Install compatible rebar3 version
        if: ${{ startsWith(matrix.otp-version, '20.') || startsWith(matrix.otp-version, '21.') || startsWith(matrix.otp-version, '22.') || startsWith(matrix.otp-version, '23.') || startsWith(matrix.otp-version, '24.')}}
        run: |
          git clone https://github.com/erlang/rebar3.git
          cd rebar3 && git checkout 3.15.2 && ./bootstrap && ./rebar3 local install
          echo "$HOME/.cache/rebar3/bin" >> $GITHUB_PATH