-
Notifications
You must be signed in to change notification settings - Fork 0
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
PHP Compat to 8.3 #26
Conversation
https://github.com/mailchimp/wordpress/blob/ddcdcf7aeb7d095d311da7ac76b3b6fe9b326074/phpcs-compat.xml#L17C1-L17C43 might also need to be bumped here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me but in testing, received one PHP warning when submitting the form: Undefined array key "phone_format" in /oss/app/public/wp-content/plugins/mailchimp/mailchimp.php on line 992
@dkotter I resolved this and another issue with options. One thing that I have noticed through testing this last bit of feedback is that we should be testing with multiple Mailchimp accounts. I have added custom merge vars for testing on my own 10up Mailchimp account. Even after not including the phone field in the form, I do not see the reported error. The OSS Mailchimp account does not have those additional merge vars, and it does throw that error. Additionally, I found an issue where some settings are not saving properly due to strict comparisons and the output returned from |
Description of the Change
Closes #3
How to test the Change
Environments
wp-config constants:
Steps:
Acceptance
Changelog Entry
Credits
@nateconley
Checklist: