Rails env for empty string env vars #27399
Conversation
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @matthewd (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review. Please see the contribution instructions for more information. |
|
@matthewd Travis and Code Climate both display the lovely green color now. Let me know if there's anything missing for this PR :) |
| *Daniel Deng* | ||
|
|
||
| * Make `Rails.env` fall back to `development` when `RAILS_ENV` and `RACK_ENV` is an empty string. | ||
|
|
There was a problem hiding this comment.
Your name should go on the bottom instead of the top of the change notice.
| @@ -78,6 +78,18 @@ def restore_default_config | |||
| end | |||
| end | |||
|
|
|||
| test "Rails.env falls back to development if RAILS_ENV == '' and RACK_ENV == nil" do | |||
There was a problem hiding this comment.
With tests I think it's usually better to write out what you're expecting versus writing code. So in that case this would be "Rails.env falls back to development if RAILS_ENV is blank and RACK_ENV is nil"
|
@eileencodes thanks for your review comments, they make sense to me. Please see the extra commit e3bcbdc . Cheers! |
|
hm... there's a travis error for mysql2, not sure what happened? I don't think it's relevant to my changes though, coz I can't see the commit e3bcbdc breaking anything. Could anyone help and have a look at that error please? Thanks heaps! |
|
@sinogermany restarted the failing build, is green now |
Thanks @maclover7 ! Cheers, Daniel |
| @@ -67,7 +67,7 @@ def root | |||
| # Rails.env.development? # => true | |||
| # Rails.env.production? # => false | |||
| def env | |||
| @_env ||= ActiveSupport::StringInquirer.new(ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "development") | |||
| @_env ||= ActiveSupport::StringInquirer.new(ENV["RAILS_ENV"].presence || ENV["RACK_ENV"].presence || "development") | |||
There was a problem hiding this comment.
presence is not present in this file. We need to require it. The test suite is passing because some of part of the test suite is requiring it and it is being used.
There was a problem hiding this comment.
Thanks @rafaelfranca . Nice catch! I've required the file now, please see a1c0cca . Cheers!
@rafaelfranca done. Thanks for the reminding :) |
|
Now you have two commits again 😄. Don't worry with conflicts after you squash, I can solve them. |
LOL I removed the conflict merging commit, hopefully I haven't done something that bothers you agian :-D |
…-env-vars Rails env for empty string env vars
…-env-vars Rails env for empty string env vars
…NV` is an empty string Follow up of rails#27399.
The story:
I've got a weird issue where
Rails.application.config_for(:app_config)returnsnil.So I looked into the console and found out that
Rails.envis an empty string.That was because I'm using
docker-composeand it's settingRAILS_ENVto an empty string because I specified indocker-compose.ymlbut not set in my local env vars.That was an easy fix on my side but I always thought rails handles that edge case.
That's why I'm submitting this PR to give rails the ability to fall back to
developmentmode when this happens.The changes:
I've added 2 tests with
RAILS_ENVandRACK_ENVbeing set to an empty string andRails.envshould bedevelopment. Also added the message toCHANGELOG.md.P.S.
This is my first commit to rails repo and I tried to follow the guidelines for contributing. I skipped some steps that seems irrelevant but if I'm missing any step please let me know.
Daniel