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

Upgrade dotenv to v3 #74

Merged
merged 4 commits into from
Feb 27, 2019
Merged

Upgrade dotenv to v3 #74

merged 4 commits into from
Feb 27, 2019

Conversation

fitztrev
Copy link
Contributor

@fitztrev fitztrev commented Feb 26, 2019

There's an incompatibility with Laravel 5.8 and this because of vlucas/phpdotenv between each's composer.json.

Project Dependency Version
laravel/framework vlucas/phpdotenv ^3.3
beyondcode/laravel-self-diagnosis vlucas/phpdotenv ~2.5

I know this PR's current state is not the best way to fix it (unless you want to release a new major version), as it will break backwards compatibility, but here's a first attempt to hopefully help at least get it started. This fixes it for 5.8.

composer.json Outdated Show resolved Hide resolved
@tvbeek
Copy link
Contributor

tvbeek commented Feb 26, 2019

I think this should result in a version 2.0 because of the backwards compatibility break. And also #72 need to be merged to make it Laravel 5.8 compatible. See: https://laravel.com/docs/5.8/upgrade#string-and-array-helpers

@crynobone
Copy link

crynobone commented Feb 26, 2019

Why not support both version?

If ^3.3 is installed, you can check against existent of Dotenv\Environment\FactoryInterface (which is not available in 2.3+). This allows you to actually tests against multiple version of Laravel.

It's very sketchy that you claim to support "illuminate/support": "5.2.*|5.3.*|5.4.*|5.5.*|5.6.*|5.7.*|5.8.*" but only test against "orchestra/testbench": "^3.8" which would only resolved to Laravel 5.8.

@fitztrev
Copy link
Contributor Author

Updated with backward compatibility. CI is now passing against Laravel 5.6 and 5.8.

It could probably be a minor release now.

@tvbeek
Copy link
Contributor

tvbeek commented Feb 27, 2019

It's very sketchy that you claim to support "illuminate/support": "5.2.*|5.3.*|5.4.*|5.5.*|5.6.*|5.7.*|5.8.*" but only test against "orchestra/testbench": "^3.8" which would only resolved to Laravel 5.8.

Maybe it is a good idea to create a PR to test for the different versions, but because #60 doesn't get any reaction I'm not sure that @mpociot will place some focus on testing as long as it works.

@fitztrev good update.

@mpociot
Copy link
Member

mpociot commented Feb 27, 2019

Thank you very much for the work on this! :)

@mpociot mpociot merged commit 57d8707 into beyondcode:master Feb 27, 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.

None yet

4 participants