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

[2.1] Move loader creation to the constructor #102

Merged
merged 1 commit into from
Jul 14, 2015

Conversation

louim
Copy link
Contributor

@louim louim commented Jul 13, 2015

This allow the use of $dotenv->required to validate environment variables without loading without explicitly loading a .env file. Unless i'm horribly mistaken, this should fix #101 and allow validation environment variables even if we don't use a .env file.

@GrahamCampbell
Copy link
Collaborator

Could you include a test case that failed before this change please?

This allow the use of "required" to validate variables without loading
without explicitely loading a .env file.

Add test case for required validation without loading
@louim louim force-pushed the allow_validation_without_load branch from 420f804 to 631e266 Compare July 13, 2015 23:55
@louim
Copy link
Contributor Author

louim commented Jul 13, 2015

@GrahamCampbell Done.

GrahamCampbell added a commit that referenced this pull request Jul 14, 2015
Move loader creation to the constructor
@GrahamCampbell GrahamCampbell merged commit 65e5231 into vlucas:master Jul 14, 2015
@GrahamCampbell
Copy link
Collaborator

🍻

@louim louim deleted the allow_validation_without_load branch July 14, 2015 00:11
@@ -35,8 +36,6 @@ public function __construct($path, $file = '.env')
*/
public function load()
{
$this->loader = new Loader($this->filePath, $immutable = true);
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this can be removed, but there is not a test case to cover the scenario for why.

Basically, if you call overload, and then load on a different file, the same loader will be retained from overload, which will leave the immutable variable set to false (from the overload method), and the expected behavior will diverge from what this method is supposed to do. A test case needs to be added to ensure this does not happen, and this line probably needs to be added back in. @GrahamCampbell

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, if i'm getting this right, You would have the line in the constructor and in the load method, and the load method would overwrite the "default" loader with a new one?

@GrahamCampbell GrahamCampbell changed the title Move loader creation to the constructor [2.1] Move loader creation to the constructor Jan 28, 2019
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.

Validation is impossible in 2.0 release without explicitly loading .env from disk
3 participants