Skip to content
This repository has been archived by the owner on Jul 20, 2018. It is now read-only.

Added class to check for forbidden functions only in template files #189

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

ylefebvre
Copy link
Contributor

Addresses issue #23

Added new class that contains a list of file patterns for all template files, along with code that builds new patterns for all supported mime types. Use pattern array to identify all template files and check if they contain any of the forbidden functions.

Addresses issue #23
This makes slugs unique and describes where they came from.
@ylefebvre
Copy link
Contributor Author

Updated error slugs to based on conversation in PR #187.

@fklein-lu
Copy link
Contributor

Any particular reason why this check was added to the WP.com Theme Review Type?

@ylefebvre
Copy link
Contributor Author

I made it part of this type since the issue #23 specified that redirects should not be placed in template files, so this would affect all theme reviews, not just VIP theme reviews.

@joshbetz
Copy link
Contributor

joshbetz commented Dec 9, 2014

@fklein-lu do you have any specific objections?

@fklein-lu
Copy link
Contributor

I don’t have any objections against including this in the WP.com Theme Review Type, although I consider this to be an edge case.

But as far as I can see, the new check never gets added to the VIP Theme Review Type. So that would have to be done, since that’s where the check makes the most sense.

I like the template detection code, and I see other checks that would only be applicable for template files. So I wonder if we can move the code that indentifies the template files outside of this check into a method. That way it could be usable in other checks without any code duplication.

@ylefebvre
Copy link
Contributor Author

What else than line 37 in config-vip-scanner.php in this PR do you see as required to add this new check to the VIP Theme Review? I tested this code and it executed the new checks.

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

Successfully merging this pull request may close these issues.

3 participants