-
-
Notifications
You must be signed in to change notification settings - Fork 483
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
flush_rewrite_rules should be flagged #668
Conversation
When we split these sniffs out this will only apply to wpcom VIP and not Go. We will want to sniff for it on Go but just make it a warning and make sure people aren't flushing on every page load.
Just checking: what do you mean by "Go" in this context ?
As far as I can see, that would be impossible to check for via a static code parser like PHPCS. |
@jrfnl: https://vip.wordpress.com/documentation/vip-go/ And right, we could make |
@jrfnl Yes my apologies it wasn't clear, "and make sure people aren't flushing on every page load." was more to be that on VIP Go it shouldn't be done, we can only flag it as a warning and let people know they shouldn't do it on every page load and rely on a human to make the call. |
@westonruter @sboisvert Thanks for the clarification. |
// @link https://vip.wordpress.com/documentation/change-your-pretty-permalinks-or-add-custom-rewrite-rules/ | ||
'flush_rewrite_rules' => array( | ||
'type' => 'warning', | ||
'message' => '%s Using flush_rewrite_rules() is not required on WordPress.com VIP since it is done on every deploy and every theme switch. Using this will cause excessive writes to the database and slow down performance', |
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.
The error outputted will be
flush_rewrite_rules Using flush_rewrite_rules() is not required on WordPress.com VIP since it is done on every deploy and every theme switch. Using this will cause excessive writes to the database and slow down performance
What about
Using %s() is not needed on WordPress.com VIP since it is done on every deploy and every theme switch. Using this will cause excessive writes to the database and slow down performance.
Make the function dynamic, instead of required use needed or necessary, add a fullstop to the end of the second sentence.
As to VIP Go and VIP, you can easily create a custom ruleset for VIP GO where you can change the notice to a Warning and change the message. You can see how it can be done in the PR https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/633/files#diff-23b2b1d22ca2f4c52692ecf35d074f7b |
@sboisvert @tomjn Just checking - as the VIP sniffs are going to be deprecated in WPCS, can this PR be closed ? |
I'm in favour of closing this, but I think flush rewrite rules calls should flag as a warning for general WP usage, especially if it's possible to detect it's running on every page load. Perhaps a future sniff under a performance category? |
@tomjn Maybe a new issue should be opened in which this can discussed. |
When we split these sniffs out this will only apply to wpcom VIP and not Go. We will want to sniff for it on Go but just make it a warning and make sure people aren't flushing on every page load.