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

Update fakerphp. #147

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Update fakerphp. #147

merged 1 commit into from
Dec 4, 2024

Conversation

rawdreeg
Copy link
Contributor

@rawdreeg rawdreeg commented Dec 4, 2024

All Submissions:

Changes proposed in this Pull Request:

This PR fixes PHP warnings on 8.4 throw by the fakerphp package. We're upgrading fakerphp to a version with support for php 8.4

How to test the changes in this Pull Request:

  1. Make sure you're running on PHP 8.4
  2. Check this branch
  3. Run composer install
  4. Run a wc generate command and confirm the that you don't see any warning coming from the wc-smooth-generator. Note that there's still a few warnings coming from WooCommerce core and its dependencies. There's an open issue to address those: PHP 8.4 compatibility woocommerce#52081

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 run tests with your changes locally?

Changelog entry

Tweak - Upgrade fakerphp to latest version to address PHP 8.4 compatibility

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@rawdreeg rawdreeg requested a review from layoutd December 4, 2024 11:24
@rawdreeg rawdreeg self-assigned this Dec 4, 2024
Copy link
Contributor

@layoutd layoutd left a comment

Choose a reason for hiding this comment

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

LGTM! I tested all different generators with PHP 8.4.0alpha4. As mentioned, all remaining notices seem to come before WC Smooth Generator runs (WooCommerce core, Action Scheduler, WP CLI, etc.)

This doesn't make any changes that would introduce injection attack vectors, but I will note that PHPCS complains about ~62 errors or warnings across the project 👀

@rawdreeg
Copy link
Contributor Author

rawdreeg commented Dec 4, 2024

62 only ? Sure, I opened #148. WIll have a look

@layoutd
Copy link
Contributor

layoutd commented Dec 4, 2024

Thanks, probably not a priority

@rawdreeg rawdreeg merged commit 6778005 into trunk Dec 4, 2024
coreymckrill added a commit that referenced this pull request Dec 30, 2024
With the new version of the Faker library introduced in #147, the
behavior of the `realTextBetween` seems to have changed, making it more
strict in its interpretation of the input parameters. This change avoids
fatal errors by ensuring that there is always a range of 40 between the
specified minimum number of characters and the maximum, so that the
generator can always build a string with a length that falls somewhere
within the range.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants