Skip to content
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

Remove Batcache references in messaging #618

Merged
merged 2 commits into from
Jan 12, 2021

Conversation

rebeccahum
Copy link
Contributor

VIP Go does not utilize Batcache and we overwrite the messaging in the ruleset anyways, so I think we can clean it up a bit and remove the Batcache references.

#613 is related as well.

@rebeccahum rebeccahum requested a review from a team as a code owner January 11, 2021 18:18
Copy link
Contributor

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

To summarise the changes here, as I see it:

  • Original sniffs were written for WPCOM, and so mentioned Batcache.
  • The error messages were overridden in WordPress-VIP-Go ruleset to make them more generic (since Batcache isn't used there).
  • Since we no longer have any WPCOM, then the original sniff messages can be updated to be more generalised, and the ruleset overrides can be removed.

@rebeccahum rebeccahum merged commit 4e334c1 into develop Jan 12, 2021
@rebeccahum rebeccahum deleted the rebecca/cache_constraints_messaging branch January 12, 2021 15:29
@jrfnl jrfnl added this to the 2.3.0 milestone Mar 2, 2021
@jrfnl jrfnl mentioned this pull request Mar 2, 2021
@jrfnl
Copy link
Collaborator

jrfnl commented Mar 3, 2021

Reviewed after the fact.

Notes:

  • The change to the WordPressVIPMinimum.Functions.RestrictedFunctions cookies group is good, though there are some comments in the ruleset test case files which will still need to be updated.
    Note: this change will need a changelog entry as a warning has changed to error for the WordPressVIPMinimum standard.
  • While not used in the VIP-Go standard, the message text for the WordPressVIPMinimum.Variables.RestrictedVariables.cache_constraints___SERVER__REMOTE_ADDR__ error code has now also been changed.
    That is not to say that the change is incorrect, but it is a change the PR makes no mention of.

I will send in a PR shortly to fix the comments in the ruleset test case files for both changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants