NFIBrokerage / spear

A sharp EventStoreDB v20+ client backed by Mint :yum:
https://hex.pm/packages/spear
Apache License 2.0
85 stars 14 forks source link

Fix config and compile_env stuff for elixir 1.14 warnings #77

Open burmajam opened 1 year ago

coveralls commented 1 year ago

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


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

💛 - Coveralls
yordis commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

yordis commented 1 year ago

@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 commented 1 year ago

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