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

Add Erlang 24 to README and some more info on version matching #45

Merged
merged 4 commits into from
Jun 9, 2021
Merged

Add Erlang 24 to README and some more info on version matching #45

merged 4 commits into from
Jun 9, 2021

Conversation

paulo-ferraz-oliveira
Copy link
Collaborator

I see there's also a bit on "Authenticating with Postgres in Phoenix" in the README, but I'm not sure it fits there. Should we remove that example?

@eproxus
Copy link

eproxus commented Jun 8, 2021

Maybe add an example using >0 as well?

@paulo-ferraz-oliveira
Copy link
Collaborator Author

Maybe add an example using >0 as well?

Thing is even though it's possible, it's kinda discouraged by the rest of the text: "For best results, we recommend specifying exact Erlang/OTP, Elixir versions, and rebar3 versions."

Let me hear "the others"; we'll figure something out. 😄 (I've brought this issue up in the past stating that we could also have a flag for strict-version: true, in which case we'd bypass the range finding).

@eproxus
Copy link

eproxus commented Jun 9, 2021

@paulo-ferraz-oliveira

Thing is even though it's possible, it's kinda discouraged by the rest of the text: "For best results, we recommend specifying exact Erlang/OTP, Elixir versions, and rebar3 versions."

Do you know why it is discouraged? I wondered that already when reading the documentation the first time.

@ericmj
Copy link
Collaborator

ericmj commented Jun 9, 2021

You usually want strict versions in CI to make it reliable. Otherwise you would automatically upgrade to new versions which can have breaking changes or bugs, this can cause issues if your CI is part of your deployment stack or does other critical things.

@eproxus
Copy link

eproxus commented Jun 9, 2021

@ericmj Got it. Unless convinced otherwise, I would argue it is a good idea to use "latest" if you are developing a library that other people will use with perhaps newer versions than you tested with to get an early warning (of course, nothing prevents you from testing both a specific and a latest version).

@paulo-ferraz-oliveira
Copy link
Collaborator Author

I agree with Eric (we've touched this subject elsewhere before): CI needs to be repeatable, in most cases. However, if you want to also do CI for e.g. nightly builds, to check if your software is compatible with a new release, this is also a valid use case, and the one where version ranges make the most sense.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

@ericmj, any thoughts on getting rid of the Phoenix bit? I'm not sure why it's there, to start with, but it doesn't seem to fit.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

paulo-ferraz-oliveira commented Jun 9, 2021

@eproxus wrote a nice GitHub Gist on how to use the action with cache.

Over Slack, he told me he's willing to either incorporate this in the README or create a new (e.g.) EXAMPLE_USAGE.md with basically the same content as the gist (via pull request).

We could also just point to it. Thoughts?

Edit: oh, this also made me review the whole README and I'll be pushing some minor tweaks soon.
Edit 2: 0461ff9.

@ericmj
Copy link
Collaborator

ericmj commented Jun 9, 2021

@paulo-ferraz-oliveira Drop the phoenix part imo.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

@paulo-ferraz-oliveira Drop the phoenix part imo.

Done: 5b6c41f

@starbelly
Copy link
Member

Yeah, agreed on the phoenix part. It's not exactly relevant to the project. The phoenix parts are things you need to know for using phoenix and postgres in docker containers in general 👍

@paulo-ferraz-oliveira
Copy link
Collaborator Author

Cool. I'm squash-merging, then.

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