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

Symfony 7 compatibillity #631

Merged
merged 5 commits into from
Jan 7, 2024
Merged

Symfony 7 compatibillity #631

merged 5 commits into from
Jan 7, 2024

Conversation

kl3sk
Copy link
Contributor

@kl3sk kl3sk commented Jan 5, 2024

This may allow Symfony 7 support.

Older version were dropped and older PHP version.

Fell free to comment/annotate 😃

Fixes #630

Add contarint for Symfony 7
Drop Symfony 6.4 and PHP <= 8.1
@kl3sk kl3sk changed the title Symfony 7 copmpatibillity Symfony 7 compatibillity Jan 5, 2024
@kl3sk kl3sk mentioned this pull request Jan 5, 2024
Co-authored-by: Alexis Lefebvre <alexislefebvre@gmail.com>
@alexislefebvre
Copy link
Collaborator

alexislefebvre commented Jan 5, 2024

Do you know why it fails with PHP 8.2? It may be due to the fact that annotations are not working with PHP 8.2+. 😬

It may work if we convert annotations to attributes in the test entity https://github.com/liip/LiipFunctionalTestBundle/blob/master/tests/App/Entity/User.php

@kl3sk
Copy link
Contributor Author

kl3sk commented Jan 5, 2024

This what I was searching for, and agree that is a Doctrine Annotation vs Attributes conflict.

If I'm not wrong, both can be used at the same time.

I'll have a try

@alexislefebvre
Copy link
Collaborator

Please hold on, let's check something first: #630 (comment)

@kl3sk
Copy link
Contributor Author

kl3sk commented Jan 5, 2024

Ok, so what's next.

Do we close it or I can push my tries with attributes to see what happens ?

@alexislefebvre
Copy link
Collaborator

alexislefebvre commented Jan 5, 2024

@kl3sk
Copy link
Contributor Author

kl3sk commented Jan 5, 2024

LiipFunctionalTestBundle Works fine, this bundle may be at least upgradable to 7.0

The other problème is here I think.

But how say use attribute for 8.2 and annotation otherwise

I change to attribute to test it.

@alexislefebvre
Copy link
Collaborator

But how say use attribute for 8.2 and annotation otherwise

Are annotations still necessary? I'm pretty sure that attributes can be used with Symfony 5.4+.

@alexislefebvre
Copy link
Collaborator

If you want to continue to work on this subject, please open another PR where you only drop support of Symfony 4.4 (you'll have to create a branch for that). Once it will be merged, you'll be able to rebase this PR and tests will be run automatically.

@@ -15,27 +15,27 @@
}
],
"require": {
"php": "^7.4 || ^8.0",
"php": "^7.4 || ^8.0 || ^8.1 || ^8.2",

Choose a reason for hiding this comment

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

Please revert "^7.4 || ^8.0" is fine

@@ -22,6 +22,10 @@
* @ORM\Entity()
* @ORM\Table("liip_user")
*/
#[

Choose a reason for hiding this comment

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

Can you change this, to the following, and see if this fix some errors? On Php 7.4 run.

#[ORM\Entity()]
#[ORM\Table(name: "liip_user")]

- php-version: 8.1
symfony-version: "^6.4"
- php-version: 8.2
symfony-version: "^6.4"
- php-version: 8.2

Choose a reason for hiding this comment

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

Can you please add a testrun for php 8.3 as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feel free to add it in a new PR.

@alexislefebvre
Copy link
Collaborator

But how say use attribute for 8.2 and annotation otherwise

Are annotations still necessary? I'm pretty sure that attributes can be used with Symfony 5.4+.

My bad, it requires PHP 8.0+. 😬

@alexislefebvre
Copy link
Collaborator

I opened a new PR based on this one:

@alexislefebvre alexislefebvre merged commit 6bdb51b into liip:master Jan 7, 2024
3 of 7 checks passed
@alexislefebvre
Copy link
Collaborator

See the release: #630 (comment)

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.

Symfony 7 compatibility
3 participants