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

PHPStan bump to level 6 #2259

Merged
merged 9 commits into from
Apr 15, 2024
Merged

PHPStan bump to level 6 #2259

merged 9 commits into from
Apr 15, 2024

Conversation

DjordyKoert
Copy link
Collaborator

@DjordyKoert DjordyKoert commented Apr 6, 2024 •
edited
Loading

Q A
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #...

Follow-up on #2249. Bump PHPStan to level 6 with required changes.

@DjordyKoert DjordyKoert force-pushed the phpstan-level6 branch 2 times, most recently from 5a76a1b to 3e5d674 Compare April 6, 2024 16:30
@DominicLuidold
Copy link
Contributor

@DjordyKoert If you'd like, you can assign me as a reviewer once this PR is ready to be reviewed 😊

@DjordyKoert DjordyKoert marked this pull request as ready for review April 6, 2024 22:13
@DjordyKoert
Copy link
Collaborator Author

DjordyKoert commented Apr 6, 2024 •
edited
Loading

@DjordyKoert If you'd like, you can assign me as a reviewer once this PR is ready to be reviewed 😊

Github does not allow me to assign you as a reviewer so all I can give you is a friendly ping @DominicLuidold :/

It has become quite the PR because pretty much every class (if not all) required a change. I tried my best to not introduce any breaking changes so I hope that I didn't miss anything on accident. 😄

@DominicLuidold
Copy link
Contributor

Github does not allow me to assign you as a reviewer so all I can give you is a friendly ping @DominicLuidold :/

That's fine! I'll have a look at the PR and the changes within the next couple of days.

Thanks already for improving the code quality even further! 😄

DominicLuidold
DominicLuidold suggested changes Apr 13, 2024
View reviewed changes
Copy link
Contributor

@DominicLuidold DominicLuidold left a comment

Choose a reason for hiding this comment

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

Looking really good - as you said, literally touches the whole project. 😅

I've only found very minor/nit-picky things and one question. Great effort! 🎉

src/Annotation/Model.php Outdated Show resolved Hide resolved
src/Describer/RouteDescriber.php Outdated Show resolved Hide resolved
src/Model/ModelRegistry.php Outdated Show resolved Hide resolved
src/ModelDescriber/Annotations/PropertyPhpDocReader.php Outdated Show resolved Hide resolved
phpstan.dist.neon Show resolved Hide resolved
src/PropertyDescriber/PropertyDescriber.php Outdated Show resolved Hide resolved
src/PropertyDescriber/PropertyDescriber.php Outdated Show resolved Hide resolved
@DominicLuidold DominicLuidold mentioned this pull request Apr 13, 2024
Open
DjordyKoert
DjordyKoert commented Apr 15, 2024
View reviewed changes
Copy link
Collaborator Author

@DjordyKoert DjordyKoert left a comment

Choose a reason for hiding this comment

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

found some breaking changes

src/Render/RenderOpenApi.php Outdated Show resolved Hide resolved
src/Render/RenderOpenApi.php Outdated Show resolved Hide resolved
@DjordyKoert @DominicLuidold
Apply suggestions from code review
a6dbaef 
Co-authored-by: Dominic Luidold <DominicLuidold@users.noreply.github.com>
DominicLuidold
DominicLuidold approved these changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@DominicLuidold DominicLuidold left a comment

Choose a reason for hiding this comment

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

Looking good - and good catch with the BC breaks! 💯

@DjordyKoert DjordyKoert merged commit 38682dd into nelmio:master Apr 15, 2024
12 checks passed
@DjordyKoert
Copy link
Collaborator Author

Looking good - and good catch with the BC breaks! 💯

Thank you for helping me with this @DominicLuidold I really appreciate it ❤️

@DjordyKoert DjordyKoert mentioned this pull request Apr 20, 2024
Merged
DjordyKoert added a commit that referenced this pull request Apr 20, 2024
@DjordyKoert
fix: Unknown Constraint class when not using symfony/validator (#2276)
2af8c5d 
| Q | A |

|---------------|---------------------------------------------------------------------------------------------------------------------------|
| Bug fix? | yes |
| New feature? | no <!-- please update src/**/CHANGELOG.md files --> |
| Deprecations? | no <!-- please update UPGRADE-*.md and
src/**/CHANGELOG.md files --> |
| Issues | Fix #2275 <!-- prefix each issue number with "Fix #", no need
to create an issue if none exists, explain below instead --> |

#2259 accidentally
removed the `class_exists` check for symfony/validator `Constraint`.
This currently causes applications using this bundle to fail because of
the missing symfony/validator dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DominicLuidold DominicLuidold DominicLuidold approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@DjordyKoert @DominicLuidold