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

php8.3 in the party #294

Merged
merged 2 commits into from
Dec 18, 2023
Merged

Conversation

connorhu
Copy link
Collaborator

@connorhu connorhu commented Dec 15, 2023

If php8.3 is already released, let's do a test with this version.

@connorhu connorhu self-assigned this Dec 15, 2023
@connorhu
Copy link
Collaborator Author

I am not sure about what cause the single_space_around_construct warnings. This rule introduced at v3.16.0. This is not php version related problem.

@connorhu connorhu requested a review from thePanz December 15, 2023 10:19
@thePanz
Copy link
Member

thePanz commented Dec 15, 2023

It could be that the last time php-CS-Fixer was run, it was in a previous version, thus that check not "default".

Let's fix it in this PR, wdyt?

@thirsch
Copy link
Collaborator

thirsch commented Dec 15, 2023

What exactly is php-cs-fixer criticizing here? To be honest, I don't see any issues in line #1 of the mentioned files?

@alquerci
Copy link
Contributor

alquerci commented Dec 15, 2023

include is a construct

An exemple

The doc

@thirsch
Copy link
Collaborator

thirsch commented Dec 15, 2023

include is a construct

An exemple

The doc

Thanks, in the end it's not the opening php tag marked in line one.

For me, it would be ok to fix it in this pr. Wdyt, @connorhu?

@connorhu
Copy link
Collaborator Author

and it's done.
I removed the brackets. I prefer includes to look different from functions.

@thePanz thePanz merged commit 2a68033 into FriendsOfSymfony1:master Dec 18, 2023
6 checks passed
@thePanz
Copy link
Member

thePanz commented Dec 18, 2023

Thanks @connorhu
merged!

@connorhu connorhu deleted the feature/php83 branch December 18, 2023 17:23
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.

4 participants