exercism / elixir

Exercism exercises in Elixir.
https://exercism.org/tracks/elixir
MIT License
625 stars 398 forks source link

Deprecate IO.read warning in Newsletter exercise #1503

Closed lorisp1 closed 4 months ago

lorisp1 commented 4 months ago

Hi, test close_log closes the file contained in Newsletter exercise is reporting the following warning if run with Elixir 1.17.1:

* test close_log closes the file [L#86]warning: IO.read(device, :all) is deprecated, use IO.read(device, :eof) instead
  (elixir 1.17.1) lib/io.ex:157: IO.read/2
  test/newsletter_test.exs:89: NewsletterTest."test close_log closes the file"/1
  (ex_unit 1.17.1) lib/ex_unit/runner.ex:485: ExUnit.Runner.exec_test/2
  (stdlib 5.2.3) timer.erl:270: :timer.tc/2
  (ex_unit 1.17.1) lib/ex_unit/runner.ex:407: anonymous fn/6 in ExUnit.Runner.spawn_test_monitor/4

This PR changes IO.read function call as suggested by the warning.

github-actions[bot] commented 4 months ago

Thank you for contributing to exercism/elixir 💜 🎉. This is an automated PR comment 🤖 for the maintainers of this repository that helps with the PR review process. You can safely ignore it and wait for a maintainer to review your changes.

Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:


Automated comment created by PR Commenter 🤖.

angelikatyborska commented 4 months ago

Thanks!

angelikatyborska commented 4 months ago

Ah, right, we can't just merge this because tests must work in many Elixir versions... @jiegillet thoughts?

jiegillet commented 4 months ago

The :eof value seems to have been inserted in 1.13, so 1.12 is failing.

Version 1.12 doesn't get security patches anymore, I can only assume that the timing for hard deprecation is linked to that. In that light, I think we could drop support for 1.12, if it's not officially safe, it makes sense for Exercism to discourage people using it.

jiegillet commented 4 months ago

@lorisp1 could you rebase this on main? (or merge main)

angelikatyborska commented 4 months ago

Thanks again!