Align s2Clean Pro with Theme Check Guideline

[manishsuwal]

We should ensure that s2Clean complies with the WordPress Theme Check Guidelines.

Here's a detailed explanation on Theme Check: Theme Check Handbook

We use the Theme Check Plugin which we can install on WordPress. It shows us what we need to improve, remove and add.

[raamdev]

@manishsuwal Great first issue for s2Clean. :-) I agree the Theme Check Guidelines are something we should try to comply with. @jaswsinc isn't as much of a fan of complying with anything though, haha.

---

A few suggestions regarding opening GitHub Issues:

• Be sure to include a description of the issue. Even if it's just something that is similar to the title (like, "We should ensure that s2Clean complies with the WordPress Theme Check Guidelines"), that's better than having no description at all.
• Be sure to include any relevant links. For example, what if someone comes to his GitHub Issue and has no idea what "Theme Check Guideline" you're referring to? It would be helpful if you included a link to the Theme Check Handbook. You might also want to mention there is a Theme Check Plugin that can be used to check compatibility.
• Give the GitHub Issue at least one Label. In this case, I would say this GitHub Issue is referring to an enhancement (we're making the the theme better by complying with the Theme Check Guidelines), so you could label it enhancement. If you're ever unsure about what to label something, just ping me on Slack and I'll be happy to offer some suggestions. :-)

[jaswrks]

@jaswsinc isn't as much of a fan of complying with anything though, haha.

Hey, that's not true. I don't mind complying, and I generally see guidelines as a good thing to follow when it comes to design. I mean, common ground among developers leads to much stronger communities. I don't always agree with the guidelines, and I've been known to gripe, but I still go along for the ride. Not a huge issue for me.

So sorry. I didn't mean to criticize Theme Guidelines for WordPress. I agree that we should make an effort to conform whenever that's perfectly reasonable to do so. Most of the items listed here are perfectly reasonable, and I see every reason for us to do a good job at trying to hit each of these. Referencing: https://make.wordpress.org/themes/handbook/review/required/

My comment in Slack earlier...

Right. There's no need for us to align with the WordPress.org submission guidelines, because this is going to be a premium theme that we distribute.

What I mean is, if there is functionality that is desirable, even if that means stepping outside the boundaries defined by WordPress, there is no reason for us to feel as if we need to conform to the letter in every way. For instance, if WordPress would generally disapprove a theme that calls upon PHP's eval(), but we see that eval() serves a useful purpose in a product we develop, there is no reason for us to remove that functionality just because WordPress has some rule about it.

[raamdev]

@jaswsinc writes...

there is no reason for us to remove that functionality just because WordPress has some rule about it.

Except if/when we decide to create a free + premium theme where we want the free version hosted on WordPress.org. In that case, we'd need to remove any functionality that relies on eval() to pass the theme review process.

If all we ever plan to make is premium themes, then yes, I'm with you: It makes no sense to conform to everything "the WordPress.org way" because that just means unnecessarily limiting ourselves.

[jaswrks]

Except if/when we decide to create a free + premium theme where we want the free version hosted on WordPress.org. In that case, we'd need to remove any functionality that relies on eval() to pass the theme review process.

Ah. Great point. I hadn't really considered that when it comes to s2Clean, because there is no free version at the moment. However, I totally agree with you in that respect. If the plan is to release a free version then I can definitely see where a removal of eval() would be in order.

LOL ~ I'll butt-out now, because the future of s2Clean is in very capable hands already :-) Lots of ways it could be improved, and I don't necessarily need to be involved in any of those either.

Anyway, I just wanted to apologize for having made that comment about theme guidelines yesterday in chat. I did not realize at the time that it would be discussed in this particular context.

[manishsuwal]

Since there's no need for us to align with the WordPress.org submission guidelines, because this is going to be a premium theme that we distribute, as Jason said, I'll make a list of errors which we will ignore:

1. eval() is OK, so long as we carefully review its use in the source code; i.e., keep an eye on the calls to eval() with a careful eye.
2. file_put_contents() is OK, so long as writes occur in directories that are expected to be writable by the server, and for which a warning is thrown in cases where it is not writable. For example, the wp-content/cache directory should be writable by the server. In s2Clean, the wp-content/s2clean directory should be writable by the server. The use of WP_Filesystem is needed when FTP/SSH is needed to write files. As this is slower anyway (i.e., not ideal for caching), it is not a good choice for a theme/plugin whenever it's simply writing cache files.
3. The base64_decode: That's all related to the s2Clean Pro Updater. None of that code would be in a Lite version that gets published to WordPress.org, as WordPress.org has its own update mechanism.

[jaswrks]

:+1: Lookin' good :-)

[raamdev]

@jaswsinc While we're on the topic of retaining the eval() and base64_decode() functions, I just wanted to mention that there are many security software packages that will scan PHP code for those two functions and report the theme/plugin as "possibly malicious".

While that's not an issue now, we may run into a lot more reports (questions from customers) about that once we start selling s2Clean to a larger audience and people start installing it on their site. Just something to keep in mind.

[manishsuwal]

@raamdev writes...

I just wanted to mention that there are many security software packages that will scan PHP code for those two functions and report the theme/plugin as "possibly malicious". While that's not an issue now, we may run into a lot more reports (questions from customers) about that once we start selling s2Clean to a larger audience and people start installing it on their site.

Will writing about this on FAQ or KB be a possible solution? Some users will not take the risk of using a "possibly malicious" theme. What do we do of them?

[raamdev]

Will writing about this on FAQ or KB be a possible solution?

Yes, that sounds like a solution. I suggest creating a new KB Article draft for this (https://github.com/websharks/s2clean-kb/issues) and referencing this GitHub issue.

Some users will not take the risk of using a "possibly malicious" theme. What do we do of them?

True. But we can't make everyone happy.

---

I'd say this GitHub issue can be closed one the KB Article draft mentioned above has been created. I see no point in this issue holding up further progress in getting s2Clean Pro up for sale. :-)