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

Changes specification to support unexisting .env file #9

Conversation

christopher-francisco
Copy link

If this is OK, this would be kind of a breaking change, as the specification that dictates the module should throw an exception when no .env file is present would need to disappear, rendering ModuleTest unusable so far (probably the custom exceptions too).

If you have any ideas you want me to implement just say the word!

@abacaphiliac
Copy link
Owner

how about a proxy function in Module that handles this package's exception interface? for example,

a new function Abacaphiliac\ZendPhpDotEnv\Module::loadOptionalEnvironmentVariables could wrap Abacaphiliac\ZendPhpDotEnv\Module::loadEnvironmentVariables, catch Abacaphiliac\ZendPhpDotEnv\Exception\ExceptionInterface to ignore it (or log it, we can decide to do something like that later), and then change https://github.com/abacaphiliac/zend-phpdotenv/blob/master/src/ZendPhpDotEnv/Module.php#L47 to wire up your proxy method instead.

in this way, the internal factories and loaders can continue to be very specific in their function. we wouldn't need to remove the thrown exceptions, or change their tests. i expect some of the tests in ModuleTest to fail because we are changing the default behavior of the package, but i think a change similar to the one i've outlined will only break the Module class and tests.

@christopher-francisco
Copy link
Author

Sounds lovely, I'll update this PR with that behavior

@abacaphiliac
Copy link
Owner

hi @chris-fa , i just had another thought. since i'm adding configuration to this module in #11 , i can read in multiple optional .env locations from the config, which can be overridden by the application. the application can decide to specify the filepaths by env, constant, or working directory, which i think is a pretty clean inversion of control. should simplify the module class.

this idea would still be a break for the module, as i plan to support your change in default behavior. do you mind if i give that a go? i'll submit my PR to you for review.

@christopher-francisco
Copy link
Author

@abacaphiliac sounds perfect. I'll be submitting my PR this week, since I was unable last week cause I was full of work 😅 .

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.

2 participants