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

Move to PHP8.0 syntax in Components #13502

Merged
merged 3 commits into from
Jan 19, 2022

Conversation

Zales0123
Copy link
Member

Q A
Branch? 1.11
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

We can finally do that 🚀 ❤️

@Zales0123 Zales0123 added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). DX Issues and PRs aimed at improving Developer eXperience. labels Jan 18, 2022
@Zales0123 Zales0123 requested a review from a team as a code owner January 18, 2022 11:59
@Roshyo
Copy link
Contributor

Roshyo commented Jan 18, 2022

Do you really think that promoted properties improve readability ?

@loic425
Copy link
Member

loic425 commented Jan 18, 2022

Do you really think that promoted properties improve readability ?

Less code is good, I'm in favor of that great PHP 8 feature. I was wondering if we should already use it on Sylius technical packages.

Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

I'm in favour. I would say, that readability may be subjective and for sure, I'm not yet used to the new syntax. But for sure it is one place less when someone may do mistake

Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

a few more places

@lchrusciel lchrusciel merged commit d459d6c into Sylius:1.11 Jan 19, 2022
@lchrusciel
Copy link
Member

Thank you, Mateusz! 🎉

@@ -10,6 +10,8 @@

return static function (ContainerConfigurator $containerConfigurator): void
{
$containerConfigurator->import(SetList::PHP_80);
Copy link
Contributor

@TomasVotruba TomasVotruba Jan 19, 2022

Choose a reason for hiding this comment

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

Tip for easier maintenance of PHP version. There is now "up to" one liner: https://getrector.org/blog/new-in-rector-012-the-latest-php-in-a-single-import

@TomasVotruba
Copy link
Contributor

Great job with upgrade 👍

I love the diff 😆

image

lchrusciel added a commit that referenced this pull request Feb 4, 2022
…Bundle) (Zales0123)

This PR was merged into the 1.11 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.11
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | continuation of #13502
| License         | MIT



Commits
-------

50ac49c PHP 8 syntax in bundles #1
489c163 Fix line length issues
ced91b9 Typehint templating engines properly
Zales0123 added a commit that referenced this pull request Jun 24, 2022
This PR was merged into the 1.11 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.11          |
| Bug fix?        | no                                                      |
| New feature?    | no                                                      |
| BC breaks?      | no                                                      |
| Deprecations?   | no |
| Related tickets | based on #14038, continuation of #13502
| License         | MIT                                                          |

I've included here also [the PR](#13970) as it has been removed only from master, and now it was making the build failed on this PR

<!--
 - Bug fixes must be submitted against the 1.10 or 1.11 branch(the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

3acaac2 [Behat] Introduce PHP 8.0 syntax
cb6d604 [Behat] Fix CS after introducing PHP 8.0 syntax
204e6d5 [Behat] Fix arguments order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants