Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare code for Elixir 1.18 #15

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Prepare code for Elixir 1.18 #15

wants to merge 7 commits into from

Conversation

rNoz
Copy link
Collaborator

@rNoz rNoz commented Jan 19, 2025

As mentioned in PR opened by @camelpunch, this PR solves that issue while keeping the same spec as the original intention provided by the code base.

I have been reviewing the specification and other libraries (go, zig and javascript), and keeping the 0x2F (magic byte) is the best way to go, than removing it (approach taken by the mentioned PR).

Then, I have added Elixir 1.18 to the CI Test and Lint jobs (version matrix) and added --warnings-as-errors in mix compile.

Correcting badges (README) and incorporating 3 new github action workflows as a replacement for Travis.

Finally, bumping versions of all dependencies.

58 Tests and 91.5% Code Coverage

.........................................................
Finished in 0.1 seconds (0.1s async, 0.00s sync)
6 doctests, 52 tests, 0 failures
----------------
COV    FILE                                        LINES RELEVANT   MISSED
 93.8% lib/ex_image_info.ex                          341       49        3
  0.0% lib/ex_image_info/detector.ex                  10        0        0
100.0% lib/ex_image_info/types/bmp.ex                 26        6        0
100.0% lib/ex_image_info/types/gif.ex                 34        9        0
 88.8% lib/ex_image_info/types/ico.ex                 53       18        2
100.0% lib/ex_image_info/types/jp2.ex                 29        6        0
 78.9% lib/ex_image_info/types/jpeg.ex                63       19        4
100.0% lib/ex_image_info/types/png.ex                 29        6        0
 95.8% lib/ex_image_info/types/pnm.ex                 94       24        1
100.0% lib/ex_image_info/types/psd.ex                 26        6        0
 89.1% lib/ex_image_info/types/tiff.ex               123       37        4
 85.7% lib/ex_image_info/types/webp.ex                90       21        3
[TOTAL]  91.5%

@rNoz rNoz force-pushed the ex-1.18-warnings branch from 91013b4 to 0ffa372 Compare January 19, 2025 20:41
@nozalr nozalr requested a review from camelpunch January 20, 2025 08:49
@rNoz rNoz force-pushed the ex-1.18-warnings branch 5 times, most recently from 9a36d0d to 7ae5037 Compare January 20, 2025 20:17
@rNoz rNoz force-pushed the ex-1.18-warnings branch 5 times, most recently from aea3a84 to 1394fca Compare January 21, 2025 05:34
@rNoz rNoz force-pushed the ex-1.18-warnings branch from 1394fca to dac268f Compare January 21, 2025 05:35
@nozalr nozalr requested a review from warmwaffles January 21, 2025 05:40
@warmwaffles
Copy link
Collaborator

Hah, I was looking at resolving the type errors yesterday. Got sucked into another issue on a different library.

@warmwaffles
Copy link
Collaborator

Side note, is committing the generated docs useful anymore? 8 years ago it made sense, but now hexdocs is not going anywhere and regenerating them is fairly simple.

@@ -41,11 +41,13 @@ jobs:
fail-fast: false
matrix:
os: ["ubuntu-20.04"]
elixir: ["1.15", "1.14", "1.13"]
elixir: ["1.18", "1.16", "1.15", "1.14", "1.13"]
Copy link
Collaborator

@warmwaffles warmwaffles Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can drop 1.13, it was marked end of life https://endoflife.date/elixir

Edit: Same thing with OTP 24 https://endoflife.date/erlang

@camelpunch
Copy link
Collaborator

Looks good. I haven't pored over every detail in this change, but it passes the tests in my PDF generator that uses this as a dependency and eliminates the warnings.

Copy link
Collaborator

@camelpunch camelpunch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above!

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.

3 participants