hotfix remove comma

[Sidsector9]

Description of the Change

The release of 1.2.0 introduced a parse error as reported here - https://wordpress.org/support/topic/parse-error-critical-with-update-1-2-0/

This issue is not reproducible, I am guessing it is reported on a different version of PHP.

Closes #124

How to test the Change

Changelog Entry

Fixed - Parse error caused by a comma

Credits

Props @Sidsector9

Checklist:

• I agree to follow this project's Code of Conduct.
• I have updated the documentation accordingly.
• I have added tests to cover my change.
• All new and existing tests pass.

[iamdharmesh]

Thanks for working on this and fixing this. I am not able to reproduce this but changes looks good to me.

However, based on error it looks like issue reporter may using PHP < 7.3 as support for trailing comma added in PHP 7.3. This plugin requires minimum PHP 8. So, not sure if should release this in Hotfix release. It's fine to release this with our next scheduled release instead of hotfix release for only this issue.

@dkotter @jeffpaul any thoughts here?

Thanks

[jeffpaul]

Perhaps including #122 will help ensure folks are on >=PHP8?

[dkotter]

I don't think we should have a trailing comma here in the first place so I'm fine with getting this fix out.

That said, this is now the second time we've had someone be able to update one of our plugins when their PHP version doesn't match our required minimum (Safe SVG being the other). It seems the minimum PHP requirement in WordPress doesn't always work, whether that's a core issue or some conflict with various security/backup type plugins.

The way we addressed that in Safe SVG was by adding our own PHP check (see https://github.com/10up/safe-svg/blob/develop/safe-svg.php#L54). We've done the same in Distributor as well. I'd suggest we start adding this same check to all of our plugins so if someone is able to update/install when they shouldn't be able to, we are able to catch that and show a warning, rather than cause PHP errors. I would suggest we add a similar check as part of #122.

[jeffpaul]

Yeah, that unfortunately sounds like the safest path @dkotter. @vikrampm1 mind adding issues across our WP plugin repos to add our own PHP check and then move those all to To Do so we can try and get that work covered across the 10up team?

[ravinderk]

Yeah, that unfortunately sounds like the safest path @dkotter. @vikrampm1 mind adding issues across our WP plugin repos to add our own PHP check and then move those all to To Do so we can try and get that work covered across the 10up team?

@vikrampm1, did you open issues?

[vikrampm1]

@ravinderk thanks for the ping. I haven't yet, I will update back here once they are opened.

[vikrampm1]

@jeffpaul @dkotter I have opened the issues across all the WP plugin repos and added them to our to-do column as high priority, let me know if I missed anything. cc @ravinderk

[ravinderk]

@dkotter Can we release this hotfix?

[Sidsector9]

@dkotter instead of implementing the PHP in each repository, can we maintain our own composer "utility" package common for all plugins?

As for conflicts between 2 or more plugins using the same package, we can use php-scoper to avoid conflitcs?

I tested out a small package I made - https://github.com/Sidsector9/wp-utils. We can move this to a 10up repository and use it across all repos.