Make dotenv a suggestion instead of a requirement

[gmcvey-gear4music]

The composer.json for this project makes dotenv a requirement, however the actual code only uses dotenv if the class exists and if it doesn't and the proper environment is set then the package should function just fine.

I'd suggest that switching dotenv from a required dependency to a suggestion (or a require-dev dependency) so that it's not installed if you don't need it.

[lesstif]

Hi,
thanks for your suggestion.

I think that most user use Dotenv as default configuration.

The actual code you have mentioned is for distinguishing Dotenv 1 and Dotenv 2, is not optional package.

So It would be a bit of a waste for those who don't use it, but I think it would be a useful to have it default for most users.

thanks

[cdtweb]

Would like to chime in here.. I pulled this into a little cli framework I wrote, and even though I was already loading a .env file with the appropriate JIRA_HOST, JIRA_USER, and JIRA_PASS variables set, I still received the following error when calling a service class without passing ArrayConfiguration into the construct:

Fatal error: Uncaught Dotenv\Exception\InvalidPathException: Unable to read the environment file at ../.env. in /Users/xxxxx/Scripts/.bootstrap/vendor/vlucas/phpdotenv/src/Loader.php:75

Would be nice if it did not require a .env file to exist to work, and require the developer to provide their own way of setting environment variables, or bootstrapping their own dotenv loader.

[cdtweb]

I should mention that I have no problem just passing ArrayConfiguration into each server class for the time being. I do appreciate the work that has gone into this project, so thank you!

[lesstif]

@cdtweb thanks for your comment.

I'm sorry but I didn't know there was such a problem.(I just using ENV only)

i had fixed it on v1.30.1.

[brzuchal]

Personally I find it weird requiring a Dotenv package dependency. For projects like Symfony Dotenv is not used so the only reason to fetch it in many projects is only this library.

IMO making a vlucas/phpdotenv a required dependency is not needed here and could easily be set as a suggested one. Imagine service locator using services.yaml etc. where core service can be configured from multiple input ways without a need to use Dotenv.

---

The way I found to avoid downloading Dotenv package by Composer is putting that package in replace node like that in composer.json:

{
    "replace": {
        "vlucas/phpdotenv": "*"
    },
    "require": {
        "lesstif/php-jira-rest-client": "^1.44"
    }
}

Note! It requires to run composer require lesstif/php-jira-rest-client again then Composer automatically removes vlucas/phpdotenv.

@lesstif please consider moving vlucas/phpdotenv to suggest once again.

[lesstif]

@brzuchal thanks your your feedback.

last week i reviewed your great suggestion, but most code assumed that dotenv is default configuration. :(

so, moving "vlucas/phpdotenv" to suggest field is easy, but other code to need rewritten, for example unit test and example code.

[brzuchal]

@lesstif As far as I can tell additing conditional here

php-jira-rest-client/src/JiraClient.php

Line 75 in f7c3fcd

$this->configuration = new DotEnvConfiguration($path);

which checks if vendor dotenv library is available would be enough.
Another place is Larervil Service provider here

php-jira-rest-client/src/JiraRestApiServiceProvider.php

Line 30 in c816447

$this->app->bind(ConfigurationInterface::class, function () {

And at the end I see no unit test which checks that relate to dotenv configuration explicitly, so whenever phpunit tests run with vendor dotenv library it's I thinkok, but for test completeness a unit test on JiraClient which passes $config array is already available.