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 config and compile_env stuff for elixir 1.14 warnings #77

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

burmajam
Copy link

@burmajam burmajam commented Nov 2, 2022

No description provided.

@coveralls
Copy link

Pull Request Test Coverage Report for Build c0c9094b0141a2b774d46ce1030dd9f2a2353b22-PR-77

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 352bc5f38bcc77a3501f1ea07fb65a79c1d4a7da: 0.0%
Covered Lines: 688
Relevant Lines: 688

💛 - Coveralls

@yordis
Copy link
Contributor

yordis commented Nov 2, 2022

Probably just remove config/prod.exs, config/dev.exs and config/test.exs files since this is a library package and probably only config/text.exs would exist ever which would be config/config.exs :2cents:

@the-mikedavis
Copy link
Collaborator

Yeah, ideally we should remove all config/ files - I think the remaining configuration can be in-lined into the test files, either in setup/2 blocks or module attributes.

I think there will also be some coveralls ignores to apply if we want to upgrade the latest Elixir version in the matrix 1.12->1.14 because of the extra coverage that we can blame @pmonson711 for 😛

@burmajam
Copy link
Author

burmajam commented Nov 3, 2022

Unfortunately Application.compile_env! is introduced in elixir 1.10, so that one will break for 1.7 as well

@yordis
Copy link
Contributor

yordis commented Dec 10, 2022

@the-mikedavis related to Elixir versions. from Ueberauth https://github.com/ueberauth/.github/blob/master/SECURITY.md

We do our best to follow Elixir Compatibility and Deprecations, we are committed to only support the latest version of Elixir.

I am not saying you should do it, but 🤷🏻 I would stick to the idea, last 2 or 3 versions

@the-mikedavis
Copy link
Collaborator

Yeah, updating minimum Elixir version would probably be ok. I like trying to "golf" the needed Elixir version for build since I have run into scenarios in the past where a library forced my hand for updating an application's Elixir version (and that's a bit annoying because then you need to make a larger change just to update the library version than you were otherwise planning).

It would probably be ok to at least bump the minimum version we test in CI to 1.9 though so that we can use Config consistently

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