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

Add Symfony 7.2 and PHP 8.4 to test matrix #351

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

ker0x
Copy link
Contributor

@ker0x ker0x commented Dec 20, 2024

No description provided.

@yann-eugone
Copy link
Member

Hello, thank you for your contribution

IMO, that's a lot for a single PR
Probably too much

@ker0x
Copy link
Contributor Author

ker0x commented Dec 20, 2024

Hello @yann-eugone, thank you for your feedback !

I reduce the PR to only add Symfony 7.2 and PHP 8.4 to the test matrix! I will submit others PRs for the rest.

@ker0x ker0x changed the title Add Symfony 7.2 and PHP 8.4 to test matrix, add typing, fix deprecati… Add Symfony 7.2 and PHP 8.4 to test matrix Dec 20, 2024
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Show resolved Hide resolved
@ker0x ker0x force-pushed the symfony-72-php84 branch 2 times, most recently from 1bfe51a to 55c1eb6 Compare December 21, 2024 14:38
@MonsieurV
Copy link

Hi, I second this PR for the types annotations. I started getting deprecation warnings after switching to PHP8.4.

One example:

Deprecated: Presta\SitemapBundle\Event\SitemapPopulateEvent::__construct(): Implicitly marking parameter $section as nullable is deprecated, the explicit nullable type must be used instead

@yann-eugone
Copy link
Member

Hi @MonsieurV

Please feel free to open an issue or a PR on this topic

@MonsieurV
Copy link

Hi @MonsieurV

Please feel free to open an issue or a PR on this topic

Hi @yann-eugone,

I've read the submitted code by @ker0x, and it solves all the deprecations for PHP8.4 (by adding the typing annotation ?). Well, except in test code, but I think maybe he may add these last annotations to complete this PR (then all the tests should run).

See
https://github.com/prestaconcept/PrestaSitemapBundle/actions/runs/12446059174/job/35187492070?pr=351#step:5:22
for the last deprecations to fix in test code.

@yann-eugone
Copy link
Member

Fine then, but at the moment this PR cannot be merged, due to the fact that it breaks all the tests

@ker0x ker0x force-pushed the symfony-72-php84 branch from 55c1eb6 to 445cda9 Compare January 9, 2025 16:31
@ker0x ker0x requested a review from yann-eugone January 9, 2025 16:34
@ker0x
Copy link
Contributor Author

ker0x commented Jan 9, 2025

@yann-eugone I just push some changes! Can you check them please?

@ker0x ker0x requested a review from yann-eugone January 10, 2025 10:17
Copy link
Member

@yann-eugone yann-eugone left a comment

Choose a reason for hiding this comment

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

Thanks for your help 🙏

@yann-eugone yann-eugone merged commit 296be93 into prestaconcept:4.x Jan 10, 2025
9 checks passed
@ker0x ker0x deleted the symfony-72-php84 branch January 10, 2025 10:33
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