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

Allow installation with Symfony 7 #131

Merged
merged 6 commits into from
Jul 15, 2024

Conversation

alexander-schranz
Copy link
Contributor

@alexander-schranz alexander-schranz commented Jan 30, 2024

This will allow the usage of the Symfony Theme Bundle with Symfony 7. There are some blocking dev dependencies which I removed for testing Symfony 7, they are only required for the lint task so we can still make the Tests run on Symfony 7.

Bildschirmfoto 2024-05-08 um 11 14 38

The changed methods should be no bc break as they are all part of final classes.

@alexander-schranz alexander-schranz force-pushed the feature/upgrade-symfony-7 branch 3 times, most recently from 01abfcf to a30c7b1 Compare January 30, 2024 16:55
name: Remove analyse dependencies
run: |
composer remove vimeo/psalm --no-update
composer remove sylius-labs/coding-standard --no-update
Copy link
Contributor Author

@alexander-schranz alexander-schranz Jan 30, 2024

Choose a reason for hiding this comment

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

This packages are not required to run tests but for linting on the latest dependencies it is required to upgrade it: SyliusLabs/CodingStandard#60

run: |
composer remove vimeo/psalm --no-update
composer remove sylius-labs/coding-standard --no-update
composer remove rector/rector --no-update
Copy link
Contributor Author

@alexander-schranz alexander-schranz Jan 30, 2024

Choose a reason for hiding this comment

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

This packages are not required to run tests but for linting on the latest dependencies it is required to upgrade it: #132

run: composer update
name: Remove analyse dependencies
run: |
composer remove vimeo/psalm --no-update
Copy link
Contributor Author

@alexander-schranz alexander-schranz Jan 30, 2024

Choose a reason for hiding this comment

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

This packages are not required to run tests but for linting on the latest dependencies it is required to upgrade it: #133

@TheMilek
Copy link
Member

TheMilek commented May 8, 2024

Hi @alexander-schranz!
No worries, just reopened up to trigger the tests 😄

@alexander-schranz
Copy link
Contributor Author

@TheMilek Thank you, I fixed the CI part. The Analyse part should be fixed in the psalm upgrade here #133. If we can merge #132 and #133 I can rebase this one.

@ajgarlag
Copy link

ajgarlag commented Jun 5, 2024

Hi @TheMilek. Can I help in any way to get this PR merged?

@alexander-schranz alexander-schranz force-pushed the feature/upgrade-symfony-7 branch from 2ae84fb to e09a5c3 Compare July 15, 2024 12:56
@alexander-schranz
Copy link
Contributor Author

@TheMilek rebased!

@TheMilek TheMilek merged commit 0567073 into Sylius:2.4 Jul 15, 2024
6 checks passed
@TheMilek
Copy link
Member

Thank you, @alexander-schranz!

@TheMilek
Copy link
Member

@TheMilek rebased!

Merged! 🚀
Sorry for the delay in responding, I'll try to improve that. 😅

@alexander-schranz alexander-schranz deleted the feature/upgrade-symfony-7 branch July 15, 2024 13:13
@alexander-schranz
Copy link
Contributor Author

@TheMilek No problem thanks for the merge :)

@alexander-schranz
Copy link
Contributor Author

Looks like I missed changes in the ThemeCollector which where supressed by psalm annotation: #134

@CoderMaggie CoderMaggie mentioned this pull request Aug 23, 2024
74 tasks
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