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

feat: _ can be used as separators in environment variable names #5156

Merged

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Oct 1, 2021

Description
You can use both as environment variable name:

  • database.default.hostname (now) - looking for it first
  • database_default_hostname (new) - if not found, looking for it

See #4026

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

database.default.hostname -> database_default_hostname
See codeigniter4#4026
Because fixture `.env` was changed.
@MGatner
Copy link
Member

MGatner commented Oct 2, 2021

The code looks good. Could you share your motivation behind this change? Are there other frameworks that use underscores? Or perhaps deployment solutions? Or do you have an underscore fetish? 😉

@samsonasik
Copy link
Member

samsonasik commented Oct 2, 2021

never use _ usage, what if user has existing _ config as a part of key value?

SimpleConfig.onedeep_value=baz

as onedeep_value as the key itself. is that previously working or after this change, that will still works?

@kenjis
Copy link
Member Author

kenjis commented Oct 2, 2021

@MGatner Some environments can't use environment variables with dot.
See #4026

And I saw this tweet:
https://twitter.com/eclairtom/status/1443603184597962752
English Translation:

Versioned up to #Docker desctop 4.1.0. docker-compose is no longer usable.
The .env used by CodeIgniter4 is now affected. Because it uses constants that contain dots. (4.0 ignored.)
I have to change the folder structure for docker at worst.

@kenjis
Copy link
Member Author

kenjis commented Oct 2, 2021

@samsonasik Good question!
I think property name like onedeep_value is not used by CI4,
but users might use it.

I added a test case, and it passed.

@kenjis kenjis force-pushed the feat-underscore-as-env-var-name-separator branch from c1dfd53 to b5255ed Compare October 3, 2021 00:15
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

I think this is probably the right direction to solve the problem with dot variables for some environments.

The only thing missing is documentation. We don't want this to become a secret feature, so it needs to be documented in the user guide.

@kenjis
Copy link
Member Author

kenjis commented Oct 3, 2021

@michalsn Thanks! I added it to the documentation.

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Nice teamwork! This looks ready to me.

@kenjis kenjis requested a review from samsonasik October 4, 2021 00:55
@kenjis kenjis merged commit 6e34d68 into codeigniter4:develop Oct 6, 2021
@kenjis kenjis deleted the feat-underscore-as-env-var-name-separator branch October 6, 2021 00:55
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