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

Update 0002-release-schedule.md regarding PHP engine changes #12

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

colinmollenhour
Copy link
Member

See discussion here

@kiatng
Copy link

kiatng commented Feb 23, 2025

I prefer the release schedule as is because:

  1. it's simpler, v21 means PHP8. Sure, composer won't allow update, but I use git and composer with ignore platform (my local environment has different PHP ver from the staging and production servers). It's not a strong argument since I just need to remember that v20.14.0 is now PHP8+.
  2. the works that were put into the next branch (should be v21) should not be discarded and should be continue, for example, removal of prototype.js

But I won't block this PR.

@sreichel
Copy link

2. the works that were put into the next branch (should be v21) should not be discarded and should be continue, for example, removal of prototype.js

O/c it should not be discarded!

Recently I have backported Contacts-Form formkey. There is an open PR for the RWD-changes. OpenMage/magento-lts#4267. There is https://github.com/sreichel/openmage-bc-mysql4-classes for the removed mysql4-classes. All changes made to admin-theme (remove prototype.js) could be ported 1:1. (i'd not care abot BC here).

But there are also changes that need to be reviewd/rewrote ....

@sreichel
Copy link

  1. it's simpler, v21 means PHP8. Sure, composer won't allow update, but I use git and composer with ignore platform (my local environment has different PHP ver from the staging and production servers). It's not a strong argument since I just need to remember that v20.14.0 is now PHP8+.

Thats why i use DDEV. To make sure local version matches production/staging. This shouls make "ignore-platform" obselete?

@kiatng
Copy link

kiatng commented Feb 24, 2025

Thats why i use DDEV.

We have a different workflow, not sure how DDEV can be incorporated. Anyway, OpenMage should accommodate different update workflows.

Again, it's just my personal preference to not change.

@colinmollenhour
Copy link
Member Author

I think using --ignore-platform-reqs is already a problem as recently I used it to update the demo server. It was running PHP 8.0, I had PHP 8.1 locally. This allowed Composer to install a version of symfony/css-selector that was too new for the production server and I got the syntax errors. So, even if OpenMage doesn't adopt this versioning strategy, other projects already have. Therefore, I don't think we should give any consideration to supporting the use of --ignore-platform-reqs since it's already broken/unsafe - I'll have to adjust my workflows to avoid using it as well as this is a recent revelation for me... 😄

@sreichel
Copy link

sreichel commented Feb 25, 2025

with ignore platform (my local environment has different PHP ver from the staging and production servers)

Why? :)

Looks like https://docs.openmage.org/developers/tools/ddev/ misses the most important. The config.xml.

Maybe you can explain your workflow?

For me

  • git clone MyNewProject
  • run ddev setup comand
  • adjust setting in ddevs config.yml to match production/staging
name: my-new-project
type: magento
docroot: ""
php_version: "7.4"
webserver_type: nginx-fpm
xdebug_enabled: false
additional_hostnames: []
additional_fqdns: []
database:
    type: mariadb
    version: "10.4"
use_dns_when_possible: true
composer_version: "2"
  • start ddev
  • db-import
  • run composer install
  • done ...?

@colinmollenhour
Copy link
Member Author

colinmollenhour commented Feb 25, 2025

In the case of the demo instance, I was running into errors on Github Actions and just wanted to get the OpenMage code updated - I knew what version I wanted, but as I was using my native "php", I was getting errors due to various PHP extensions not being installed. So, I used --ignore-platform-reqs to ignore those, not thinking about the PHP engine version and dependencies like "symfony/css-selector" that install different versions based on the engine running composer.. Also I didn't understand that I should have just updated the config.platform and I would have been ok to do that... I added a section to our docs for others here.

Normally, I use Docker, so my local environment is the same as the remote, but for anyone not using Docker (or by extension DDEV), changing PHP versions is a PITA and not everyone is a composer expert. I agree that everyone (including me 😨) should use containers all the time.. I went for years without installing php on my dev machine, I think I eventually had to install it to use some IDE I was trying out.. then I occasionally used local composer instead of installing composer in containers.. that led me to this mistake. Surely I'm not the only one that will be bitten by this kind of error, but as it is already out of our control, I think that's a risk we don't need to protect users from.

So @kiatng I think for your workflow, the solution is to make sure that your composer config platform.php X.X matches your production environment, and then you would be safe with the minimum PHP version changing between minor versions.

I get the argument that v20=PHP7, v21=PHP8, etc.is easy to remember, but I think this is the "old" way and we need to let it go and let composer manage the dependencies because that is exactly what it was built for as long as knuckleheads like me use it properly. :)

@colinmollenhour
Copy link
Member Author

colinmollenhour commented Mar 6, 2025

@kiatng another issue with matching PHP versions is that it requires our schedule to be in sync with theirs to a large degree, which in practice will mean that we just skip or delay major version upgrades for the sake of avoiding new major version numbers..

I think as long as we announce the dropping of older PHP versions in the release notes it will work out well for everyone: free to update at any time, stores not broken by "composer update" (unless they are using Composer wrong), no overhead for us to manage new major versions. Example:

This release drops support for PHP 7.4 and 8.0. If you are using these versions, please update your environment to PHP 8.1, 8.2 or 8.3 and update your Composer file's config.platform accordingly to ensure you receive the correct dependencies.

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.

3 participants