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

PHP 8.0 syntax in bundles (without CoreBundle and ApiBundle) #13562

Merged
merged 3 commits into from
Feb 4, 2022

Conversation

Zales0123
Copy link
Member

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

@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 28, 2022
@Zales0123 Zales0123 requested a review from a team as a code owner January 28, 2022 13:55
@probot-autolabeler probot-autolabeler bot added the Admin AdminBundle related issues and PRs. label Jan 28, 2022
@@ -47,7 +41,7 @@ public function validate($value, Constraint $constraint): void
$propertyPath = $this->context->getPropertyPath();

foreach (iterator_to_array($this->context->getViolations()) as $violation) {
if (0 === strpos($violation->getPropertyPath(), $propertyPath)) {
if (str_starts_with($violation->getPropertyPath(), $propertyPath)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

OH MY GOD it feels so good ❤️

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I agree, this is a great one.

@Zales0123 Zales0123 force-pushed the php8-syntax-bundles branch from 242fe71 to f08ca1c Compare January 28, 2022 14:04
@Zales0123 Zales0123 changed the base branch from master to 1.11 January 28, 2022 14:05
@Zales0123 Zales0123 changed the base branch from 1.11 to master January 28, 2022 14:05
@Zales0123 Zales0123 force-pushed the php8-syntax-bundles branch from f08ca1c to 2def894 Compare January 28, 2022 14:07
@probot-autolabeler probot-autolabeler bot added the Maintenance CI configurations, READMEs, releases, etc. label Jan 28, 2022
@Zales0123 Zales0123 changed the base branch from master to 1.11 January 28, 2022 14:08
lchrusciel added a commit that referenced this pull request Feb 3, 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 | extracted from #13562 for better readability
| License         | MIT

Commits
-------

b21b626 PHP 8 syntax in bundles vol.3
8b42dbd CS fixes
@Zales0123 Zales0123 force-pushed the php8-syntax-bundles branch 2 times, most recently from 8437ecf to c4e7e4b Compare February 3, 2022 14:44
UserProviderInterface $userProvider,
private UserImpersonatorInterface $impersonator,
private AuthorizationCheckerInterface $authorizationChecker,
private UserProviderInterface $userProvider,
?RouterInterface $router,
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
?RouterInterface $router,
private ?RouterInterface $router,

Copy link
Member

Choose a reason for hiding this comment

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

But to be honest, we should rework this argument, to be able to replace logic with the new one

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting idea, we can elaborate over it in the separate PR 🖖

Comment on lines 29 to 30
private ?RouterInterface $router;

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
private ?RouterInterface $router;


$this->impersonator = $impersonator;
$this->authorizationChecker = $authorizationChecker;
$this->userProvider = $userProvider;
$this->router = $router;
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
$this->router = $router;

@Zales0123 Zales0123 force-pushed the php8-syntax-bundles branch from c4e7e4b to 9e395ac Compare February 3, 2022 15:48
@Zales0123 Zales0123 force-pushed the php8-syntax-bundles branch from 9e395ac to ced91b9 Compare February 3, 2022 15:56
@lchrusciel lchrusciel merged commit 5723c0b into Sylius:1.11 Feb 4, 2022
@lchrusciel
Copy link
Member

Thank you, Mateusz! 🥇

@Zales0123 Zales0123 deleted the php8-syntax-bundles branch February 4, 2022 10:42
lchrusciel added a commit that referenced this pull request Feb 4, 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 | extracted from #13562 for better readability
| License         | MIT

Commits
-------

128117c PHP 8 syntax in bundles vol.2
e52f894 Line length fixes
160361f Small CS fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin AdminBundle related issues and PRs. DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance CI configurations, READMEs, releases, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants