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 to php 7.4 #365

Merged
merged 1 commit into from
Oct 3, 2022
Merged

upgrade to php 7.4 #365

merged 1 commit into from
Oct 3, 2022

Conversation

stephanvierkant
Copy link
Contributor

  • Bumped minimum PHP version to 7.4
  • Added types to private properties (and removed phpdoc if superfluous)
  • Used arrow functions

Fixes #357

src/Knp/Menu/Matcher/Voter/RouteVoter.php Outdated Show resolved Hide resolved
@stof
Copy link
Collaborator

stof commented Jan 4, 2022

this also needs to remove the CI job running on PHP 7.3 (the lowest_deps job must not be removed, but updated to PHP 7.4 instead)

Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

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

Can you remove "dev" suffix from sf 5.4 and 6.0 in the build? Also, the "dev deps" should be on PHP 8.1

Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

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

It's OK for me if we manage to solve the last failing test

@garak
Copy link
Collaborator

garak commented Mar 17, 2022

@stephanvierkant any news on this?

@Chris53897
Copy link
Contributor

Thanks for your work @stephanvierkant
Maybe you can make that change to get this PR merged.

The failing tests are because of the deprecation messages with --prefer-lowest argument.
The solution is to bump minimum version of twig to 1.40 => 1.43
"twig/twig": "^1.43 || ^2.9 || ^3.0"

I did bump "psr/container": "^2.0", as we now have php 7.4 (but this is optional)
https://packagist.org/packages/psr/container#2.0.2

I suggest to update phpunit-bridge as well. (php: >=7.1.3) (but this is optional)
"symfony/phpunit-bridge": "^6.0",

https://github.com/Chris53897/KnpMenu/blob/php74/composer.json

Testrun:
https://github.com/Chris53897/KnpMenu/actions/runs/2338296768

@stof
Copy link
Collaborator

stof commented May 17, 2022

I did bump "psr/container": "^2.0", as we now have php 7.4

Dropping support for v1 is a bad idea. There is no reason for that in term of maintenance, and it increases the risk of dependency conflicts.

Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

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

LGTM

@garak garak merged commit e479ce0 into KnpLabs:master Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop support for PHP 7.3?
4 participants