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

[WIP] Add Symfony support version 7.0, drop unmaintainned versions #206

Closed
wants to merge 9 commits into from

Conversation

lchrusciel
Copy link
Member

@lchrusciel lchrusciel commented Dec 9, 2023

Best from #203, #205, but in highly opinionated fashion

Waiting for Behat/Behat#1442 to be merged. Works nicely with that PR
image

#SymfonyHackday

@damien-louis
Copy link

Hi @lchrusciel Behat/Behat#1442 just merged.

DOCUMENTATION.md Outdated Show resolved Hide resolved
DOCUMENTATION.md Outdated Show resolved Hide resolved
@damien-louis
Copy link

@lchrusciel except code review, everything is ok to merge?

@lchrusciel lchrusciel changed the title Add Symfony support version 7.0, drop unmaintainned versions [WIP] Add Symfony support version 7.0, drop unmaintainned versions Dec 9, 2023
@lchrusciel
Copy link
Member Author

lchrusciel commented Dec 9, 2023

@damien-louis, we should wait for Behat's release of the new version and then remove the WIP commit. Right now I'm testing it against master branch to assert correctness of current solution

@@ -48,6 +43,12 @@ jobs:
- name: Lock Symfony version
run: VERSION=${{ matrix.symfony-version }} .github/workflows/lock-symfony-version.sh

- name: Remove Symfony proxy-manager-bridge
run: |
composer config --no-plugins allow-plugins.dealerdirect/phpcodesniffer-composer-installer true
Copy link
Contributor

Choose a reason for hiding this comment

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

the allowed plugins should be configured in your composer.json directly to avoid bothering contributors each time.

@@ -48,6 +43,12 @@ jobs:
- name: Lock Symfony version
run: VERSION=${{ matrix.symfony-version }} .github/workflows/lock-symfony-version.sh

- name: Remove Symfony proxy-manager-bridge
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this in the job for Symfony 7 hides the fact that this will still be broken for users of the package.

Removing packages in some CI jobs is an acceptable workaround for dev requirements, but not for normal requirements.

My suggestion here would be to drop support for Symfony 5.4 as well (keeping those users on the previous minor version of the extension) as Symfony 6.2+ does not need the proxy-manager-bridge to actually make services lazy.

"symfony/http-kernel": "^4.4 || ^5.3 || ^6.0",
"symfony/proxy-manager-bridge": "^4.4 || ^5.3 || ^6.0"
"php": "^8.1",
"behat/behat": "dev-master",
Copy link
Contributor

Choose a reason for hiding this comment

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

this should stay as ^3.6.1 IMO. Your CI job for Symfony 7 should be configuring minimum-stability: dev for now until the support is part of a Behat release.

@@ -70,7 +71,7 @@ jobs:
with:
coverage: none
ini-values: "memory_limit=-1, zend.assertions=1"
php-version: 7.4
php-version: 8.1
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use at least PHP 8.2 so that it can install the latest deps

@Yozhef
Copy link
Contributor

Yozhef commented Jan 11, 2024

Already drop php 7 and Symfony 4,5 support - https://github.com/FriendsOfBehat/SymfonyExtension/releases/tag/v2.5.0

@Yozhef Yozhef closed this Jan 11, 2024
@lchrusciel lchrusciel deleted the sf7 branch January 30, 2024 09:45
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.

5 participants