search

Getbeans / Beans

Beans WordPress Theme Framework. The default branch is set to development, please switch to the master branch for production.
https://www.getbeans.io
Other
392 stars 61 forks source link

Fix - WPCS 1.10 will cause errors and warnings #341

Closed lrobi2014 closed 6 years ago

lrobi2014 commented 6 years ago

Hi @christophherr

I re-did the issue - but there is one integration test that keeps failing on PHP 5.6. Can you help me here?

christophherr commented 6 years ago

Hi @lrobi2014,

looks like your last PR introduced a g at the very beginning of class-beans-image-editor.php. https://d.pr/i/Xh49mk Somehow that slipped through the tests and me :) Please just add a commit to this PR correcting it.

lrobi2014 commented 6 years ago

@christophherr Oh thank you! I missed this

christophherr commented 6 years ago

FYI, there is one more sniff that needs to be renamed in lib/api/utilities/polyfill.php New name: PHPCompatibility.FunctionUse.NewFunctions.array_replace_recursiveFound Old name: PHPCompatibility.PHP.NewFunctions.array_replace_recursiveFound

lrobi2014 commented 6 years ago

@christophherr Renaming the sniff in lib/api/utilities/polyfill.php throws a php 5.2 incompatibility error. Should we modify the ruleset to increase the minimum supported version?

christophherr commented 6 years ago

@lrobi2014 My mistake, I was testing the next version of phpcompatibility. :) Please revert the change of the phpcompatibility sniff name.

And no, we cannot change the minimum required PHP version as that has the potential to break existing websites.

lrobi2014 commented 6 years ago

@christophherr Ok, no problem will do

lrobi2014 commented 6 years ago

@christophherr I see there is a PR addressing the failing integration test. I guess we wait until that one is merged before we progress with this one?

christophherr commented 6 years ago

@lrobi2014 Yes, let's get the other PR in first.