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

Major dudestack version update - Moving to PHP dotenv 5.3, roots/wp-config, oscarotero/env #4

Merged
merged 22 commits into from
Feb 18, 2021

Conversation

ronilaukkarinen
Copy link
Member

For stability and security these packages need to be updated. More to why here (phpdotenv), here (oscarotero/env) and here (roots/wp-config).

Practically this is getting dudestack to this decade. This was inspired by new PHP versions that cause problems with composer v1 and need for composer v2 support. After all, development of v1 stopped in 2020-06-03. PHP dotnev version we used was dated back in 2019-01-05...

This update brings:

  • More modern and simplified definitions under config folder
  • Support for PHP 7+
  • Stability and future proof
  • More DRY methods
  • Support for Composer 2

Good things are:

  • WPackagist already supports these
  • wp-languages already supports these
  • Tested and working out of the box with createproject macos-lemp

Breaking changes:

  • This will break your project if are using composer v1 (check it out with composer --version)
  • ACF Pro Installer will no longer work as it supports only v1 (see issue) but on the other hand it's not updated after 2017 and needs to be replaced anyway, we have documented new way in dudestack README of this PR
  • For legacy projects you need to replace files under config/, update composer packages (composer/installers, wp-cli/wp-cli-bundle, johnpbloch/wordpress, oscarotero/env, roots/wp-config)
  • Not tested, needs proof that it works (or under discussion should we dump vagrant): marlin-vagrant

Please note

To be fully tested the new version will be needed to push to packagist as it doesn't support branches. Currently composer create-project (that is bundled with createproject) pulls the latest release. This has not been released yet so we cannot fully test it before doing so. I have run tests manually and confirmed this works fine though if composer packages are up to date and dev branch files are in use.

@ronilaukkarinen ronilaukkarinen mentioned this pull request Feb 16, 2021
composer.json Outdated
Comment on lines 67 to 68
"oscarotero/env": "^2.1",
"roots/wp-config": "^1.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be on top of the requires right after phpdotenv. Otherwise, these are easily removed as a plugins.

Copy link
Member Author

Choose a reason for hiding this comment

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

True. Going ahead and correcting this.

README.md Outdated
* Basic knowledge about bash scripting, deployment with capistrano, npm packages, bundle, composer etc.
* Vagrant ([marlin-vagrant](https://github.com/digitoimistodude/marlin-vagrant)) OR [osx-lemp-setup](https://github.com/digitoimistodude/osx-lemp-setup) but can be configured for MAMP or even Docker (in planning)
* GitHub account
* Unix-based OS (built for Mac OS X by default)
* Access to staging and production servers that supports sftp and git
* Projects located under $HOME/Projects
* Git
* PHP >= 5.6
* Ruby >= 2.2
* PHP >= 7.1
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only partially true, because when using pivvenit/acf-pro-installer PHP 7.2 is required. It might not be used by users outside of our agency, but for our devs we need that package and thus 7.2

Copy link
Member Author

Choose a reason for hiding this comment

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

New phpdotenv requires at least php 7.1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also now that I checked Roots and oscarotero/env also require it. So it's pretty much a must-have minimum.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, my point was to say that we probably should state the 7.2 as minimum version instead of 7.1 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Aah, thought you are commenting about the PHP version update in general... tired brain, sorry. Yes, 7.1 is directly from those composer packages, should be 7.2 for us.

@ronilaukkarinen ronilaukkarinen merged commit ced2072 into master Feb 18, 2021
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