-
Notifications
You must be signed in to change notification settings - Fork 38
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
Proposal: new sniff categorization #166
Comments
Just thinking, there are three more sniffs we need to take into account and add to categories as these will be removed from WPCS in the near future, but are currently included in TRTCS:
For the session related sniffs, we may need a new category, but I can't currently come up with a good name. Suggestions welcome. |
I think that sessions would be good in plugin territory category. |
I would prefer to see them organized like our Requirements document, which has these headings, not all of which are possible to be checked with sniffs.
I think Language and Options can be merged with Code. That leaves a list of
|
NOTES:
|
Action needed (by @dingo-d and @pattonwebz )
|
Categories, with sniffs:
|
Why isn't We should also change this in the handbook. It wouldn't be a bad idea to update the themes handbook a bit too. |
Because the Plugins category is about including plugins or requiring plugins to work, whereas the ExcessFunctionality category is about doing things that themes should not be doing. They can both be considered Code, but they are two separate sections in the requirements (and very different). |
So the categories, as proposed by @joyously look fine, we just need to add a few more. I propose a bit modified version:
To be added later on (maybe)
Not needed (in my opinion)
We need to have a concensus about this, and this should be addressed in the Team Review meeting. |
I think using the requirements structure is a good idea. Once we have defined the categories in the ruleset we should not change them and that would apply the same for the requirements documentation. I would prefer to use The category |
Ok, so after much debate we came up with these initial categories for the existing sniffs:
The sniffs in the PR can be moved to these categories or added in the new ones if need be. |
@dingo-d Just one last question - are you sure that the |
Oh and one more question - the |
@jrfnl You mean a separate category just for sessions? We could put them in If it's not a problem it would be better to change the sniff name. |
No, I meant having both sniffs in the same category, whichever category that is. Having them in different categories feels counter-intuitive.
As what we are doing now is a complete and utter BC-break, at this moment, we have all the freedom in the world to change sniff names (and anything else) 😎
Both names you suggest repeat the |
OK, then put the sessions in the Yeah, the plugins part is redundant, since the category is called |
As the repo history has now been rewritten and the change is contained in the The remaining actions from comment #166 (comment) should still be taken though:
|
As this repo was (is) a fork from WPCS, when this project started, it was decided to put all Theme specific sniffs in a
Theme
sniff category.However, things change. And one of the things which is about to change is that the repo will be de-forked from WPCS.
This means that it can, from that point onwards, create its own logical categories.
In this issue, I'd like to propose the following initial categorization for the sniffs currently in the
Theme
category:HTML
Sniffs which examine the HTML used in a theme.
ForbiddenIframe
NoTitleTag
- alternatively, this sniff could go into theWP
category as it suggests usingadd_theme_support()
instead.Integrations
Sniffs which are related to external libraries/dependencies/services etc.
CorrectTGMPAVersion
NoAutoGenerate
PluginTerritory
Sniffs which check that themes do not add functionality (which should be done via plugins).
NoAddAdminPages
PluginTerritoryFunctions
WP
Sniff which check that native WP functionality is used instead of re-inventing the wheel
FileInclude
NoFavicon
More categories
... can be added if and when needed in the future.
Impact Analysis
proposed category
section.👉 Opinions welcome. If there are no objections, the above categorization will be used when the de-forking PR is created.
The text was updated successfully, but these errors were encountered: