gatekeeper icon indicating copy to clipboard operation
gatekeeper copied to clipboard

$envPath in loadConfig

Open Swader opened this issue 10 years ago • 2 comments

I suggest moving the check

        $envPath = ($envPath !== null) ? $envPath : getcwd();

from the loadConfig method to the loadDotEnv method, and throwing a proper exception if the path was specified but loading failed. That calls for an exception, rather than just false, because something unexpected occurred. It's not a case for a soft fallback to false. Took me a while to debug what was happening as I encountered this.

I believe the loadDotEnv method is a better home for this check, considering that's the only method in which the actual parsing of the file and loading of the config values actually happens.

I realize, however, that this would be a BC break.

Thoughts? Should I submit a PR anyway?

Swader avatar Aug 06 '15 19:08 Swader

I think throwing the exception is okay for a break like this. It would only throw if the file wasn't found - a negative case - so I'm good with this change.

enygma avatar Aug 12 '15 00:08 enygma

Right, but the exception isn't specific and debugging it is hard. Could we at least have it throw out completely or return an error object of some kind? Rather than catching it and returning false? False really isn't helpful in debugging.

Swader avatar Aug 12 '15 07:08 Swader