Skip to content

Conversation

@rebeccahum
Copy link
Contributor

Fixes #370.

@rebeccahum rebeccahum requested a review from GaryJones February 12, 2019 06:15
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.

Should this be added to the WordPressVIPMinimum ruleset test, to check that it does through a warning for that ruleset?

@rebeccahum
Copy link
Contributor Author

I don't think so because that grouping of rules isn't explicitly listed in the WordPressVIPMinimum ruleset.

@GaryJones
Copy link
Contributor

I don't think so because that grouping of rules isn't explicitly listed in the WordPressVIPMinimum ruleset.

They may be implicitly included by virtue of the whole directory of sniffs being included, but that doesn't mean we shouldn't be testing for them. There are plenty of // WordPressVIPMinimum. instances in the ruleset test, including a whole block for RestrictedFunctions; though it looks incomplete, any new additions should be added anyway IMO.

@rebeccahum
Copy link
Contributor Author

If we are going to test for them, we should test for all being implicitly included and re-work the WordPressVIPMinimum integration tests to account for that in a different PR.

@GaryJones
Copy link
Contributor

If we are going to test for them, we should test for all being implicitly included and re-work the WordPressVIPMinimum integration tests to account for that in a different PR.

Right - the ruleset test needs updating in another PR, but the bit relevant to this PR should be added in this PR - think atomic commits / merges.

@rebeccahum rebeccahum force-pushed the rebecca/add_site_option branch from 9af2bd1 to c413719 Compare February 12, 2019 18:06
@rebeccahum
Copy link
Contributor Author

Fair enough, done!

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.

PHPCS Audit has been updated.

@GaryJones GaryJones merged commit e74e30a into master Feb 12, 2019
@GaryJones GaryJones added this to the 1.0.0 milestone Feb 12, 2019
@GaryJones GaryJones deleted the rebecca/add_site_option branch February 12, 2019 21:18
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