-
Notifications
You must be signed in to change notification settings - Fork 381
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
Move validation background tasks behind feature flag filter until Site Scanning is implemented #5892
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #5892 +/- ##
=============================================
- Coverage 75.16% 75.12% -0.04%
- Complexity 5665 5667 +2
=============================================
Files 210 210
Lines 17019 17023 +4
=============================================
- Hits 12792 12789 -3
- Misses 4227 4234 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Plugin builds for f12c5d5 are ready 🛎️!
|
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.
On second thought, I think the related tests should be updated accordingly.
Updated tests in f12c5d5. |
QA Passed Getting a list of scheduled cron events with After activating a plugin with the following filter: add_filter( 'amp_temp_validation_cron_tasks_enabled', '__return_true' ); I verified that the
To verify that the add_filter( 'pre_schedule_event', function ( $pre, $event ) {
error_log( 'Scheduling: ' . $event->hook );
return $pre;
}, 10, 2 ); When Developer Tools is disabled and I go to publish a post, I can see that the line |
I've been thinking about the new cron jobs for checking site validation, specifically these introduced in #5515 for #1756:
amp_validate_urls
cron which checks a sample set of URLs.amp_single_post_validate
cron which checks a URL after a post is published and the user doesn't have DevTools enabled.At the moment, these two cron jobs are responsible for generating validation data which we are not yet using. We do not have the aforementioned notification center to communicate AMP Site Health and we haven't yet implemented Site Scanning as part of the onboarding wizard. Validation data is being generated even when DevTools is disabled now. This causes a problem also because we don't have any garbage collection of validation data (#4779). Therefore, since we aren't using the output of these additional validation checks, I propose that we put them behind a feature flag (internal filter) and turn them off for 2.1. This will allow us to punt a couple more issues to 2.2 to coincide with Site Scanning which is what they are the foundation for. Specifically #5750 and #4779.