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

Properly support current Laravel versions #37

Merged
merged 1 commit into from
Jun 7, 2020
Merged

Properly support current Laravel versions #37

merged 1 commit into from
Jun 7, 2020

Conversation

sergejostir
Copy link
Contributor

This PR does the following:

  1. It properly implements deferrable service provider, for Laravel 5.8+ (https://laravel.com/docs/5.8/upgrade#deferred-service-providers).

  2. It properly specifies dependencies in composer.json. It also does not use greater or equal to sign (>=) anymore, since considering the semver it is not neccessary that all future Laravel versions will simply work.

The result is that this package is now compatibile only to Laravel 5.8 and newer. That shouldn't be a problem since all earlier versions have reached EOL long time ago already, and if we want to keep this package up to date to current standards that's a small price to pay.

I have also updated README to reflect these changes. The only thing left is to update the Github repository description to say "The .env file editor tool for Laravel 5.8+".

@JackieDo JackieDo merged commit 16d796e into JackieDo:master Jun 7, 2020
@JackieDo
Copy link
Owner

JackieDo commented Jun 7, 2020

@sergiz : I agree with what you said. At the same time I need to consult from you the following:

  • Firstly, from now on, we should release new versions with 2.x tags, in order to completely separate from the old releases?
  • Second, I plan to move the documentation of this package to using Github Page (put on the gh-pages branch) as I did with another of my package here. Should I do that?

@sergejostir
Copy link
Contributor Author

a) Considering the semver, there were no incompatible API changes introduced, so it should be a minor release. Here is the article that is often linked: https://www.doctrine-project.org/2017/07/25/php-7.1-requirement-and-composer.html
I guess it should only be noted in the README, what version should people install when using <L5.8.

b) Well, I personally prefer a single page documentation using README for simple packages like this. Your cart package seems pretty complex, so it makes sense to make a multi page docs, but for env-editor where we only have a few methods to document, I find it easier to have everything on a single page.
Also it's easier for people, that are searching for packages, if everything is shown up on the first page, so they can quickly see what it can do, without opening new tabs.

@JackieDo
Copy link
Owner

JackieDo commented Jun 7, 2020

a) Considering the semver, there were no incompatible API changes introduced, so it should be a minor release. Here is the article that is often linked: https://www.doctrine-project.org/2017/07/25/php-7.1-requirement-and-composer.html
I guess it should only be noted in the README, what version should people install when using <L5.8.

b) Well, I personally prefer a single page documentation using README for simple packages like this. Your cart package seems pretty complex, so it makes sense to make a multi page docs, but for env-editor where we only have a few methods to document, I find it easier to have everything on a single page.
Also it's easier for people, that are searching for packages, if everything is shown up on the first page, so they can quickly see what it can do, without opening new tabs.

OK, so we will come to the following agreement:

  • Only new releases with 1.2.x tags
  • Keep the documentation in the README file (but need to standardize the content more carefully).

Thank you for your advice.

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