Fix 1.10 Supervisor deprecation warnings

[filipecabaco]

Fixed all deprecation warnings derived from the usage of :simple_one_for_one strategy

[CrowdHailer]

@filipecabaco thanks, this looks great.

Could you add a changelog entry. Does this work as a patch update to the package version

[filipecabaco]

Not quite due to test dependencies. I've tested in 1.6 due to the fact that it's the current Ace minimum elixir requirement and ex_doc and earmark require elixir 1.7.

I tried to push ex_doc version down with no effect 😅so we'll have this warning constantly although it doesn't seem to affect anything.

ex_doc needed elixir 1.7 from 0.18 onward but earmark I wasn't able to find the release where they started requiring it.

How I tested

From the Ace project folder:

docker run -v $(pwd):/ace -e MIX_ENV=test -it elixir:1.6-otp-21-alpine sh

Command:

cd ace && \
mix local.hex --force && \
mix deps.get && \
mix test && \
mix format --check-formatted && \
mix dialyzer --halt-exit-status

Maybe the way to go is actually change the Ace elixir requirement to 1.7 but that would no longer be a patch 😓meaning that Changelog would need a bump in minor.

What's your opinion on this?

[CrowdHailer]

We had some problems with dialyzed on earlier Elixir versions, that we never fixed.

At the time we thought it was too soon to drop older support.

However that was at least a year ago. I am quite happy to drop versions and make the next breaking release. If we do that I would suggest make the minimum version 1.9 so it's not too soon when we next have this discussion.

On Mon, 16 Mar 2020, 11:53 Filipe Cabaço, notifications@github.com wrote:

Not quite due to test dependencies. I've tested in 1.6 due to the fact that it's the current Ace minimum elixir requirement and ex_doc and earmark require elixir 1.7.

I tried to push ex_doc version down with no effect 😅so we'll have this warning constantly although it doesn't seem to affect anything.

ex_doc needed elixir 1.7 from 0.18 onward but earmark I wasn't able to find the release where they started requiring it. How I tested From the Ace project folder:

docker run -v $(pwd):/ace -e MIX_ENV=test -it elixir:1.6-otp-21-alpine sh Command:

cd ace && \

mix local.hex --force && \

mix deps.get && \

mix test && \

mix format --check-formatted && \

mix dialyzer --halt-exit-status

Maybe the way to go is actually change the Ace elixir requirement to 1.7 but that would no longer be a patch 😓meaning that Changelog would need a bump in minor.

What's your opinion on this?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CrowdHailer/Ace/pull/141#issuecomment-599494609, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMXHHUQU26YXWN5QSJJY2LRHYHKVANCNFSM4LLRXX2Q .

[filipecabaco]

I can tackle that in this PR and as a result push minor to 0.19.0. I'll also take the opportunity to bump up dependency versions with mix deps.update --all.

[CrowdHailer]

Great, that should help us close down some of the long standing pull requests that we have

On Mon, 16 Mar 2020, 12:33 Filipe Cabaço, notifications@github.com wrote:

I can tackle that in this PR and as a result push minor to 0.19.0. I'll also take the opportunity to bump up dependency versions with mix deps.update --all.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CrowdHailer/Ace/pull/141#issuecomment-599510926, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMXHHSBER5U32P64I3NCJDRHYMAVANCNFSM4LLRXX2Q .

[filipecabaco]

CI failed in dializer step 😓 but I had the same issue with the other commit on this PR...

I don't have any working theory's on why it could be failing in Travis and not locally (classic "it works on my machine") .

Does anyone have any idea on what it could be so I can try and debug it / fix it?

[CrowdHailer]

Is it just me that can't see CI integrating with github? Have they changed that but I can't see anything on this PR which normally shows the status of tests.

What OTP version are you using we should update that in travis if also update Elixir

[filipecabaco]

Can't see the status either... Here's the config I've updated in the last commit

https://travis-ci.org/github/CrowdHailer/Ace/jobs/663107446/config

language: elixir
elixir:
  - 1.9.4
otp_release:
  - 21.3
env:
  - MIX_ENV=test
before_script:
  - mix local.hex --force && mix local.rebar --force && mix deps.get
script:
  - mix test
  - mix format --check-formatted
  - mix dialyzer --halt-exit-status
cache:
  directories:
    - _build
before_cache:
  # should only keep the dialyzer artifacts
  - mix clean
  - mix deps.clean --all

Needed to add mix local.rebar --force

[filipecabaco]

Ok, after bump there's actually an error on travis

https://travis-ci.org/github/CrowdHailer/Ace/builds/666081967

lib/ace/http2/connection.ex:37:no_return
Function init/1 has no local return.

Which is weird... will try to debug it a bit more

[CharlesOkwuagwu]

Please why is this not merged?

[filipecabaco]

Issues in CI 😅

Not sure why though...

https://travis-ci.org/github/CrowdHailer/Ace/jobs/669637405

[CrowdHailer]

Merged, because all the tests pass.

I would accept a PR for github actions. I seem to have lost track of my access to travis, also if dialyzer is not simple to fix a PR to remove that would be helpful.

[developess]

@CrowdHailer Do you have an idea of when this minor update will be pushed to Hex? Apologies if I'm missing some context - looked around at the various issues and PRs and can't see any info on that

[CrowdHailer]

Looks like there was no reason 0.19 was not pushed to hex. it's in the changelog. I will push it now