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

Expand Build Matrix #150

Closed

Conversation

jherdman
Copy link
Contributor

Let's also trust Travis to do the right thing with respect to selecting
the OTP version to use.

Let's also trust Travis to do the right thing with respect to selecting
the OTP version to use.
@@ -27,4 +18,4 @@ sudo: false
script:
- mix test
- mix credo
- if [[ `elixir -v` = *"1.6"* ]]; then mix format --check-formatted; fi
- if [[ `elixir -v` = *"1.7"* ]]; then mix format --check-formatted; fi
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change this to 1.7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.7, being the latest public release (outside of 1.8-rc1), felt like the logical choice to run this particular build step on — especially since I'm adding it to the build.

Copy link
Member

Choose a reason for hiding this comment

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

This is meant to ensure the command isn't run without formatter (versions prior to 1.5).

- 1.6
- 1.5
- 1.4

otp_release:
Copy link
Member

Choose a reason for hiding this comment

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

Can you link us to the documentation that explains why we don't need this? If you look around pretty much every Elixir project has these OTP releases defined, including Elixir itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I believe so. As I understand it, if we leave this off, it will pick one OTP version. We are intentionally testing against multiples because it is common in real-world situations to have Elixir and OTP on different versions.

@doomspork
Copy link
Member

@jherdman I'm going to close this PR for now. None of these changes will work as expected:

  1. We intentionally want to test against multiple OTP versions, not let Travis pick just one.
  2. The 1.6 guard is to ensure the formatter is only run for versions in which it exists (so not 1.3, 1.4, and 1.5). It should not be updated with each new release.

Next time feel free to open an issue first to discuss changes not currently documented 👍

@doomspork doomspork closed this Dec 29, 2018
@doomspork doomspork mentioned this pull request Dec 29, 2018
@jherdman jherdman deleted the expand-travis-matrix branch December 30, 2018 15:20
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.

2 participants