env variables are strings

[dvorakluk]

In Bash, environment variable is always a string (which is what getenv() returns).
Because of the way this constant is checked in WP:
if ( defined('DISABLE_WP_CRON') && DISABLE_WP_CRON ),
setting DISABLE_WP_CRON=false would resolve to true.
This approach should be more predictable

[swalkinshaw]

Could simplify this to:

define('DISABLE_WP_CRON', getenv('DISABLE_WP_CRON') == 'true');

[dvorakluk]

Of course, I just thought that the longer version is easier to understand.

[austinpray]

swalk's version is easier to understand for me

[swalkinshaw]

@dvorakluk just had one more minor comment. Would you be able to squash the commits into 1 if possible?

[dvorakluk]

@swalkinshaw Here you are :)

[swalkinshaw]

Thanks!

[andrewfrankel]

Noticed something similar recently... but should it be "True" (capital T)? That's how booleans render for me in the .env file.

SOME_VAR="True"

[aaemnnosttv]

@swalkinshaw - Shouldn't boolean environment variables should be set as an integer for this reason?

The if ( defined('CONSTANT') && CONSTANT ) conditional check is used in many places in WordPress.

It's true that defining CONSTANT=false in .env would return a false positive, but defining CONSTANT=0 would be evaluated correctly.

The way it is now, if I would define DISABLE_WP_CRON=1 in my .env, it would evaluate to false because "1" does not equal "true", and for that matter neither does "0".

I suggest reverting this change because I think it is more in line with the conventions followed in other configuration files to use 1 or 0 for boolean values. 1 and 0 are also going to be interpreted correctly by both WP and bash. Also, as @andrewfrankel pointed out, 1 and 0 will not be misinterpreted simply because of a case-sensitive difference.

I think it would be better to document the best practice for setting boolean values in the .env file rather than making this check simply for the semantic value.

In summary:

false === (bool) "0"
true  === (bool) "1"
true  === (bool) "false"
true  === (bool) "true"

[dvorakluk]

$ VAR=1 php -r "var_dump((bool) getenv('VAR')=="true");"
bool(true)
$ VAR=0 php -r "var_dump((bool) getenv('VAR')=="true");"
bool(false)

Case-sensitivity is another story, good point.

[swalkinshaw]

Think we're just going to use https://github.com/oscarotero/env and let it take care of this.