braintree / braintree_php

Braintree PHP library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
545 stars 225 forks source link

Bugfix/serializable validation errors #312

Open mjervis opened 2 years ago

mjervis commented 2 years ago

Summary

Checklist

Config value "show_progress" added successfully php ./vendor/bin/phpcs --config-set colors 1 Using config file: /braintree-php/vendor/squizlabs/php_codesniffer/CodeSniffer.conf

Config value "colors" added successfully php ./vendor/bin/phpcs --config-set php_version 70300 Using config file: /braintree-php/vendor/squizlabs/php_codesniffer/CodeSniffer.conf

Config value "php_version" added successfully php ./vendor/bin/phpcs --standard=phpcs.xml --report=summary -s lib tests ............................................................ 60 / 279 (22%) ............................................................ 120 / 279 (43%) ............................................................ 180 / 279 (65%) ............................................................ 240 / 279 (86%) ....................................... 279 / 279 (100%)

Time: 4.52 secs; Memory: 66.01MB

php ./vendor/bin/phpunit --testsuite unit --do-not-cache-result PHPUnit 9.5.13 by Sebastian Bergmann and contributors.

............................................................... 63 / 360 ( 17%) ............................................................... 126 / 360 ( 35%) ........................................................PHP Deprecated: Return type of Braintree\Error\Validation::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /braintree-php/lib/Braintree/Error/Validation.php on line 50 ....... 189 / 360 ( 52%) ............................................................... 252 / 360 ( 70%) ............................................................... 315 / 360 ( 87%) ............................................. 360 / 360 (100%)

Time: 00:00.372, Memory: 12.00 MB

OK (360 tests, 1077 assertions)



- [x ] I understand that unless this is a Draft PR or has a DO NOT MERGE label, this PR is considered to be in a deploy ready state and can be deployed if merged to main

<!-- **For Braintree Developers only, don't forget:**
- [ ] Does this change require work to be done to the GraphQL API? If you have questions check with the GraphQL team.
- [ ] Add & Run integration tests -->
crookedneighbor commented 2 years ago

Though this warning in the console seems like it should be addressed:

PHP Deprecated: Return type of Braintree\Error\Validation::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /braintree-php/lib/Braintree/Error/Validation.php on line 50

mjervis commented 2 years ago

Fixed the code style issues, not sure how I got those in TBH , I'd run the linter.

How did you get the console error? Running the unit tests and the tests doesn't throw these up?

I've fixed it inline with the other JsonSerializeable methods in the existing codebase.

hollabaq86 commented 1 year ago

hey folks, thanks for the PR! We'll take a look and provide feedback/merge.

For braintree folks, ticket 2047