-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add recommended combinations of Elixir and Erlang/OTP versions to CI #100
base: master
Are you sure you want to change the base?
Conversation
883204e
to
4d80f09
Compare
ffa0272
to
de13ec0
Compare
@keathley @garthk @Qqwy @wojtekmach I am looking forward to hearing your thoughts on this pull request. |
de13ec0
to
8a69747
Compare
8a69747
to
7e4293e
Compare
As I believe I’ve already stated elsewhere, Im not a fan of dense CI build matrices by default, I’d only reserve them for projects that would really benefit from them (eg elixirls) and for all the others, I’d test against just oldest and just newest supported version. This way we minimize resource usage that was given to us free of charge. Regarding warnings as errors in tests, the best solution I found is the following: # test/test_helper.exs
if System.get_env(“CI”) do
Code.compiler_options(warnings_as_errors: true)
end This isn’t obvious and I should have an article about it next week or so :) |
Oh, handy. I've been struggling with the full CI matrix on some other projects. I wish I'd thought of running just the oldest Elixir on the oldest OTP and the youngest Elixir on the youngest OTP. It'll deliver most of the benefit in a fraction of the resources. Thanks! |
Agreed, with one little caveat (this has bitten me in the past): Sometimes it might be worthwhile to also test newest Elixir with one-but-newest OTP and one-but-newest Elixir with the newest OTP. Sometimes this interplay will result in (compilation) errors you might otherwise miss. |
This pull request makes sure all recommended Elixir and Erlang/OTP combinations are run on CI.
Quoting José Valim:
Please refer to the original discussion on dashbitco/broadway#188 for the full rationale.
Additional functionality: make sure there are no compilation warnings, both on application and on tests, and checks formatting.
Finally, solved some minor unused variable warnings, courtesy of improved checks of the Elixir 1.11.0 compiler, and credo strict minor stylistic issues.
In case one wonders why not use
otp: N.x
for OTP as well as for Elixir, please refer to actions/setup-elixir#45.