Skip to content

Use new Supervisor syntax for elixir version >= 1.5#77

Open
dlee-tt wants to merge 2 commits intogutschilla:masterfrom
dlee-tt:master
Open

Use new Supervisor syntax for elixir version >= 1.5#77
dlee-tt wants to merge 2 commits intogutschilla:masterfrom
dlee-tt:master

Conversation

@dlee-tt
Copy link
Copy Markdown

@dlee-tt dlee-tt commented Oct 28, 2020

Hello,

I came across an issue where, after upgrading to elixir 1.11, I saw that pdf_generator.ex was still using the deprecated syntax for Supervisor.spec.worker.

I made the change to use the new child specification syntax that is applicable for elixir versions 1.5 and above.

If anything is out of place or not the best practice, I would really appreciate it if you let me know, as I'm a newbie in the open source/elixir world.

Thank you very much!

Dongkeun

@gutschilla
Copy link
Copy Markdown
Owner

Hi @dlee-tt ,
thanks a bunch for your PR! I was very busy with private things and at work so sorry this module seemed like unmaintained 😬

Right now, PdgGenerator only required Elixir 1.1 to work and thus relies on a quite a few very-style mechanics. I would assume there's not a lot of people out there still using such an old Elixir version but I am unsure if bumping the minimum required version to, let's say 1.5 is fine to everyone.
Probably yes. What's your opinion on that?

@dlee-tt
Copy link
Copy Markdown
Author

dlee-tt commented Feb 3, 2021

Hi @gutschilla,

Thank you for responding during a busy time!

In my opinion, it would be fine; as you mentioned, 1.1 is quite old at this point and I believe most users should have upgraded to version 1.5 or higher at this point in time.
And if it forces some users to upgrade their Elixir version, I think it might be preferable to do it now than later, since eventually this would be inevitable.

@maennchen
Copy link
Copy Markdown

Elixir 1.15 was released in 2017. I think requiring ~> 1.15 should not be a problem for anyone at this point. Currently, even security fixes are only supported from 1.9.

https://hexdocs.pm/elixir/main/compatibility-and-deprecations.html

@yoonwaiyan
Copy link
Copy Markdown

Hi @gutschilla, can you please review the merge request again and release the changes? I'm using Elixir version 1.14 and I have this deprecation message

@gangstead
Copy link
Copy Markdown

@gutschilla I offer a suggestion: You can raise the minimum elixir version, merge this pr, bump the package to 2.0 and put the new elixir version as the breaking change in the release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants