Skip to content

Update to php8 and SF6 #64

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 29, 2022
Merged

Update to php8 and SF6 #64

merged 1 commit into from
Apr 29, 2022

Conversation

qdurillon-dev
Copy link
Contributor

Compatibilité PHP8 et SF6

Suppression du atoum-bundle qui n'est pas dispo en php8

Correction des deprecated

MAJ de php cs-fixer

@qdurillon-dev qdurillon-dev requested a review from a team as a code owner April 14, 2022 14:53
DocRoms
DocRoms approved these changes Apr 14, 2022
View reviewed changes
FloFrad
FloFrad reviewed Apr 15, 2022
View reviewed changes
Fabex
Fabex reviewed Apr 16, 2022
View reviewed changes
@shavounet shavounet mentioned this pull request Apr 25, 2022
Closed
@qdurillon-dev
Copy link
Contributor Author

@FloFrad @Fabex Si c'est ok pour vous est ce que l'un de vous peut merge/créer un tag ? ça nous débloquerait pour la migration de notre côté 🙂🙏

shavounet
shavounet approved these changes Apr 25, 2022
View reviewed changes
@qdurillon-dev
Copy link
Contributor Author

Updated the PR with @shavounet comments

shavounet
shavounet reviewed Apr 26, 2022
View reviewed changes
Oliboy50
Oliboy50 reviewed Apr 26, 2022
View reviewed changes
Copy link
Member

@Oliboy50 Oliboy50 left a comment

Choose a reason for hiding this comment

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

thanks but our CI does not start on your PR, I will try to fix this

@Oliboy50
Copy link
Member

@kdurillon I made this PR to fix our CI

I'm sorry but you will have to wait for it to be merged, then you'll have to rebase your PR on master, and update the CI script to work on php 8.0 and php 8.1 🙏

@Oliboy50
Copy link
Member

Oliboy50 commented Apr 26, 2022 •
edited
Loading

👋 the CI has been fixed on master branch

please rebase your branch on top of master and update this file to drop support for php 7.4 and gain support for both php 8.0 and php 8.1 (to match your composer.json updates) 🙏

@Fabex
Copy link

Fabex commented Apr 26, 2022

Quand tu auras bien tout rebase par rapport à master ca sera ok pour merge et release une nouvelle version majeur

@qdurillon-dev qdurillon-dev force-pushed the php8-and-sf6 branch 4 times, most recently from bf01a39 to fb34ba9 Compare April 27, 2022 06:41
@qdurillon-dev
Copy link
Contributor Author

I had to change min versions of predis, atoum and cs-fixer to ensure the CI passed with prefer lowest on 8.1.
Should be all good now

Oliboy50
Oliboy50 reviewed Apr 27, 2022
View reviewed changes
composer.json
"symfony/symfony": "~5.0",
"psr/cache": "~1.0"
"psr/cache": "~1.0",
"friendsofphp/php-cs-fixer": "^3.4"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"friendsofphp/php-cs-fixer": "^3.4"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Min version for php 8.1, see this failed ci : https://github.com/BedrockStreaming/RedisBundle/actions/runs/2232153155

Copy link
Member

Choose a reason for hiding this comment

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

ok, we probably have to fix this directly in m6web/php-cs-fixer-config then

it is fine to merge as is and remove it later when this will be fixed 👌

Oliboy50
Oliboy50 approved these changes Apr 27, 2022
View reviewed changes
Copy link
Member

@Oliboy50 Oliboy50 left a comment

Choose a reason for hiding this comment

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

thanks :)

composer.json
"symfony/symfony": "~5.0",
"psr/cache": "~1.0"
"psr/cache": "~1.0",
"friendsofphp/php-cs-fixer": "^3.4"
Copy link
Member

Choose a reason for hiding this comment

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

ok, we probably have to fix this directly in m6web/php-cs-fixer-config then

it is fine to merge as is and remove it later when this will be fixed 👌

@Oliboy50
Copy link
Member

I don't know much about the predis API but my mates approved and I'm sure you know what you're doing, so I'll merge your PR, thanks again

@Oliboy50 Oliboy50 merged commit 3517bdd into BedrockStreaming:master Apr 29, 2022
@Oliboy50
Copy link
Member

released in v7.0.0

@qdurillon-dev qdurillon-dev deleted the php8-and-sf6 branch May 10, 2022 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shavounet shavounet shavounet approved these changes

@FloFrad FloFrad FloFrad approved these changes

@Fabex Fabex Fabex approved these changes

@Oliboy50 Oliboy50 Oliboy50 approved these changes

+1 more reviewer

@DocRoms DocRoms DocRoms approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@qdurillon-dev @Oliboy50 @Fabex @FloFrad @shavounet @DocRoms