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.

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

Upgrade phpstan to level 2 #14053

Merged
merged 13 commits into from Oct 21, 2022
Merged

Upgrade phpstan to level 2 #14053

merged 13 commits into from Oct 21, 2022

Conversation

lruozzi9
Copy link
Contributor

@lruozzi9 lruozzi9 commented Jun 3, 2022

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets partially #14019
License MIT

Still in draft, PR not completed.

@probot-autolabeler probot-autolabeler bot added API APIs related issues and PRs. Shop ShopBundle related issues and PRs. labels Jun 3, 2022
@lruozzi9 lruozzi9 force-pushed the upgrade-phpstan-to-level-2 branch from 25a4897 to 96031ab Compare June 6, 2022 08:18
@lruozzi9
Copy link
Contributor Author

lruozzi9 commented Jun 6, 2022

Almost ready. Needs SyliusLabs/PolyfillSymfonyFrameworkBundle#3 to be completed.
I'm not sure if to exclude the paths of the PHP files in the tests directories or add them to the baseline.

@lruozzi9 lruozzi9 mentioned this pull request Jun 6, 2022
Open
8 tasks
@lruozzi9 lruozzi9 marked this pull request as ready for review September 5, 2022 16:23
@lruozzi9 lruozzi9 requested a review from a team as a code owner September 5, 2022 16:23
@lruozzi9 lruozzi9 requested a review from Ferror September 5, 2022 16:24
@Ferror
Copy link
Contributor

Ferror commented Sep 15, 2022

@lruozzi9 can you please resolve conflicts?

@lruozzi9 lruozzi9 force-pushed the upgrade-phpstan-to-level-2 branch 2 times, most recently from 620b3cf to 826c3ec Compare September 16, 2022 08:26
@TheMilek
Copy link
Member

Hi @lruozzi9!
Thank you for Your contribution!
We see still some builds are failing, are you still pushing this forward?
cheers 🍻

@lruozzi9
Copy link
Contributor Author

Hello @TheMilek! After giving Psalm some blows with a hammer now looks ok!

@probot-autolabeler probot-autolabeler bot added the Admin AdminBundle related issues and PRs. label Oct 7, 2022
@Rafikooo Rafikooo added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Oct 20, 2022
GSadee
GSadee approved these changes Oct 21, 2022
View reviewed changes
src/Sylius/Behat/Client/ResponseChecker.php
@@ -79,6 +80,9 @@ public function isDeletionSuccessful(Response $response): bool

public function hasAccessDenied(Response $response): bool
{
if (!$response instanceof JWTAuthenticationFailureResponse) {
return false;
}
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
}
}

Copy link
Member

Choose a reason for hiding this comment

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

@lruozzi9
Still actual, though why it wasn't caught by CS checks in the first place.

@GSadee GSadee merged commit 3583263 into Sylius:1.12 Oct 21, 2022
@GSadee
Copy link
Member

GSadee commented Oct 21, 2022

Thanks, Lorenzo! 🥇

@lruozzi9 lruozzi9 deleted the upgrade-phpstan-to-level-2 branch October 21, 2022 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diimpp diimpp diimpp left review comments

@GSadee GSadee GSadee approved these changes

@NoResponseMate NoResponseMate NoResponseMate approved these changes

@Ferror Ferror Ferror approved these changes

@Rafikooo Rafikooo Rafikooo approved these changes

Assignees
No one assigned
Labels
Admin AdminBundle related issues and PRs. API APIs related issues and PRs. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Shop ShopBundle related issues and PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@lruozzi9 @Ferror @TheMilek @GSadee @diimpp @NoResponseMate @Rafikooo