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

Fix warnings on Elixir v1.19 #460

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Conversation

josevalim
Copy link
Contributor

No description provided.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 758a438d00d8cc22fd9e2fcdcd76de54629a5a6a-PR-460

Details

  • 17 of 27 (62.96%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 87.56%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/mint/http.ex 8 18 44.44%
Totals Coverage Status
Change from base Build 6b6f1d4049daadf5fb062a7caf47d5ea4719fb17: 0.0%
Covered Lines: 1281
Relevant Lines: 1463

💛 - Coveralls

@whatyouhide
Copy link
Contributor

Why did we go with this instead of the macro approach?

@josevalim
Copy link
Contributor Author

Because the macro doesn't work :)

@whatyouhide
Copy link
Contributor

@josevalim could you elaborate on why?

@josevalim
Copy link
Contributor Author

I just doesn't work, converting into a macro would be this:

case data do
  %Foo{} -> Foo
  %Bar{} -> Bar
  %Baz{} -> Baz
end.something(data)

So the type system will still see you are trying to invoke Foo with something that could be %Baz{}. We would need a macro that expanded to this:

case data do
  %Foo{} -> Foo.something(data)
  %Bar{} -> Bar.something(data)
  %Baz{} -> Baz.something(data)
end

Which is basically the same as conn_apply anyway.

@whatyouhide
Copy link
Contributor

Aaah gotcha ok. Thanks 🙃

@whatyouhide whatyouhide merged commit 63ce63c into elixir-mint:main Jan 27, 2025
3 checks passed
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.

4 participants