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

Add PHPStan #568

Merged
merged 9 commits into from
Jun 30, 2023
Merged

Add PHPStan #568

merged 9 commits into from
Jun 30, 2023

Conversation

HardeepAsrani
Copy link
Member

@HardeepAsrani HardeepAsrani commented Jun 14, 2023

All Submissions:

Changes proposed in this Pull Request:

This adds PHPStan to level 5 and changes have been made to best of my knowledge, so it will be good to have a review to make sure no changes are reverting intended code.

Also, there is one error left in PHPStan which I'm not able to realize why that particular line of code was written the way it is written so feedback there is appreciated.

Closes #566.

How to test the changes in this Pull Request:

  1. Just make sure everything works as before in Optimole plugin.
  2. Most importantly to test all the settings about including/excluding.
  3. Make sure to have debugging on and make sure there isn't anything in the debug logs.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@HardeepAsrani HardeepAsrani changed the title [WiP] Add PHPStan Add PHPStan Jun 20, 2023
@HardeepAsrani HardeepAsrani marked this pull request as ready for review June 20, 2023 19:22
@pirate-bot
Copy link
Collaborator

pirate-bot commented Jun 20, 2023

Plugin build for b41a949 is ready 🛎️!

@GrigoreMihai
Copy link
Contributor

@HardeepAsrani Regarding the error you mentioned I did some tests and the type of $width is an integer there. I used the gettype to get the type of the var.

It's safe to change the type on the function param declaration to integer for $width and it should fix the phpstan error.

inc/media_offload.php Show resolved Hide resolved
inc/lazyload_replacer.php Outdated Show resolved Hide resolved
inc/compatibilities/elementor_builder.php Show resolved Hide resolved
inc/app_replacer.php Outdated Show resolved Hide resolved
inc/app_replacer.php Outdated Show resolved Hide resolved
inc/app_replacer.php Outdated Show resolved Hide resolved
inc/compatibilities/elementor_builder.php Outdated Show resolved Hide resolved
inc/media_offload.php Show resolved Hide resolved
inc/media_offload.php Show resolved Hide resolved
inc/app_replacer.php Outdated Show resolved Hide resolved
Copy link
Contributor

@abaicus abaicus left a comment

Choose a reason for hiding this comment

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

@HardeepAsrani I think other than what @GrigoreMihai mentioned, this should be ok to go 👍🏻 Thanks for addressing all the other issues! 🚀

@HardeepAsrani HardeepAsrani linked an issue Jun 26, 2023 that may be closed by this pull request
@irinelenache
Copy link

@HardeepAsrani Tested the PR and didn't find any problem 🚀

@abaicus abaicus merged commit 60224a4 into development Jun 30, 2023
@abaicus abaicus deleted the phpstan branch June 30, 2023 13:27
@pirate-bot
Copy link
Collaborator

🎉 This PR is included in version 3.8.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Indicate that an issue has been resolved and released in a particular version of the product.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate PHPStan
5 participants