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

Allow local env files for each Rails environment, and host family. Fix dotenv file load order and overriding values. #2

Merged
merged 15 commits into from
Oct 20, 2021

Conversation

cfurrow
Copy link
Contributor

@cfurrow cfurrow commented Oct 20, 2021

Suggesting this change to be a major bump in version as it changes the underlying behavior.

The Problem

While doing some local testing with VCR, I realized my .env.local was superseding values set in .env.test. I did not want this.

  • .env.local is NOT committed to the repository, and therefore is local to my machine
  • .env.test IS committed to the repository, and shared by all devs + our CI environment.

My VCR tests were using MY local credentials in .env.local, which is not what I wanted it to do. The specs I was writing could/would fail in CI, as the credentials and VCR request matchers would be in conflict.

The solution

Allow for more fine-grained .env filenames, and make sure the test environment does not use my .env.local, but instead would rely on .env, .env.test, .env.HOST_FAMILY, .env.test.local and .env.HOST_FAMILY.local. Then I could rename my .env.local to .env.development.local and keep it out of the repo, and out of the test environment.

Added rspec tests

I noticed there was a spec directory, but no valid specs. And rspec was not a dependency of the gemspec, so I added it, plus rails, in order to get the rspec tests passing and validated.

lib/dotenv/beefy/railtie.rb Outdated Show resolved Hide resolved
Copy link

@f1sherman f1sherman left a comment

Choose a reason for hiding this comment

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

This looks like a great change to me! Thanks for fixing the specs in addition to making things a bit more predictable.

This change is technically breaking, so I'd probably recommend a major version bump. We can definitely do that in a separate PR.

lib/dotenv/beefy/railtie.rb Outdated Show resolved Hide resolved
@cfurrow cfurrow marked this pull request as draft October 20, 2021 15:18
@cfurrow cfurrow marked this pull request as ready for review October 20, 2021 16:14
Copy link

@f1sherman f1sherman left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks so much! I think we'll need to ping @wireframe to do the actual publish of the gem once this is merged.

lib/dotenv/beefy/railtie.rb Outdated Show resolved Hide resolved
@wireframe
Copy link
Contributor

this is amazing @cfurrow! Thank you for your contribution! I'll merge and push an updated release shortly!

@wireframe wireframe merged commit beda88a into betterup:master Oct 20, 2021
@wireframe
Copy link
Contributor

version 0.2.0 has been published to rubygems!

@cfurrow
Copy link
Contributor Author

cfurrow commented Oct 20, 2021

Woohoo! Thanks for merging so fast, @wireframe !

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.

3 participants