woocommerce / action-scheduler

A scalable, traceable job queue for background processing large queues of tasks in WordPress. Specifically designed for distribution in WordPress plugins (and themes) - no server access required.
https://actionscheduler.org
GNU General Public License v3.0
640 stars 117 forks source link

[Task] Codebase should pass linting checks #971

Open eddr opened 1 year ago

eddr commented 1 year ago

Improve coding standards across Action Scheduler. We should pass our own linting tests, and ideally those should intersect with the WPCS checks implemented by the WooCommerce Marketplace.

Additionally, as described in issues like this one, we should aim to get Action Scheduler's code to a point where it meets the expectations of the W.org plugin review team.


Original title:

Action Scheduler shows phpcs errors using wpcs : can't submit new extension to WC marketplace

Original description:

Hi and thanks for excellent and useful plugin!

I'm facing issues when trying to submit a new extension to the WC marketplace. The plugin uses Action Scheduler library and it causes the automatic tests to not pass WC own phpcs checks

Any clue what can be done?

Thanks

barryhughes commented 1 year ago

Internal discussion: p1690905319416299-slack-C3L7CFBRV

barryhughes commented 1 year ago

@eddr since you are creating a WooCommerce extension, you can presumably depend on WooCommerce being present (or can test for it). WooCommerce includes its own copy of Action Scheduler, and it should therefore be possible to use that from your extension (or else you can use WC()->queue() which effectively wraps it).

I acknowledge this comes with some trade-offs (ie, you may not be able to take advantage of the very latest functionality)—but it is probably the most practical path forward at present.

eddr commented 1 year ago

Hi! Thank you very much!

I am aware that the AS included in WC, yes The biggest problem for me is that the extension in mind should work even if WC if not active..

barryhughes commented 1 year ago

Gotcha ... in that case, I don't have an easy workaround I can share. I've updated the issue title and description, though, to create a task for cleaning things up within the Action Scheduler codebase (on our end, it is not likely something we can prioritize straight away, however).

eddr commented 1 year ago

Thanks

I really think it's mostly a problem with the WC extension submitting system

remcotolsma commented 1 month ago

The "Plugin Check (PCP)" plugin detects the following issue in the "Plugin Repo" category:

FILE: packages/woocommerce/action-scheduler/lib/cron-expression/CronExpression.php

Line Column Type Code Message
234 28 ERROR WordPress.DateTime.RestrictedFunctions.date_date date() is affected by runtime timezone changes which can cause date/time to be incorrectly displayed. Use gmdate() instead.

localhost_8888_wp-admin_tools php_page=plugin-check

https://github.com/woocommerce/action-scheduler/blob/652735ca113440f963872a37cffe8eff5637e126/lib/cron-expression/CronExpression.php#L234

This is a problem, especially with the latest plugin check developments:

Secondly, as of today, when you submit a new plugin to the Plugins Directory, it will first be run through Plugin Check’s Plugin Repo category. If the new plugin has an error level item in this category, the submission will be blocked from being submitted for review, until it is fixed.

https://make.wordpress.org/plugins/2024/10/01/plugin-check-and-2fa-now-mandatory-for-new-plugin-submissions/

Can this be addressed within this issue or should I create a new issue and open a PR?

barryhughes commented 1 month ago

Though that is (or was originally) a vendor library, it's one where we've already made other changes—so I think updating it again should be fine.

Can this be addressed within this issue or should I create a new issue and open a PR?

I think it would be fine to create a PR and link back to this issue ... but, please don't write Closes #971 in the PR description as we need some other work, and have some other PRs under review, that need to be wrapped first (thank you!).

crstauf commented 1 month ago

@barryhughes I've created dozens of PRs of manageable size: one PR per file. That should help you make forward progress on this issue without delay.

There are still a few more files to go, and once those are done, I'll swing back around and investigate the PRs that had checks failures.

crstauf commented 1 month ago

Phew, all PRs have been submitted 😓 , and attention brought to the check failures caused by Composer issue.

Hopefully these small PRs can be accepted quickly. 🤞

crstauf commented 1 week ago

@barryhughes FYI after all PRs have been merged, I plan to run PHPCS across the entire codebase again, as well as organize some things a bit.

crstauf commented 1 week ago

@barryhughes #1214 is my final PHPCS PR. :tada:

barryhughes commented 5 days ago

Thanks so much, @crstauf. We should be in a much better spot now. Taking a moment to itemize some further (hopefully final) to-do items before we can close this:

crstauf commented 5 days ago

Hmm, not sure how I missed the short array syntax fixes. That was the intent of my cleanup branch. 😡

barryhughes commented 5 days ago

Given the number of fixes, I think something was going to be missed 🙃