WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.53k stars 472 forks source link

Purpose and future of WordPress-Extra and WordPress-Core #1269

Open JDGrimes opened 6 years ago

JDGrimes commented 6 years ago

This ticket is a split from #1157 as suggested in https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1157#issuecomment-351753423.

History

In the beginning, there was just one WordPress ruleset. In #58 it was proposed that the sniffs be split into several subsets (something similar was also proposed in #42):

A lot of the sniffs are opinionated and may not be reflective of an actual problem. We should have a ruleset XML file that just contains the core sniffs for checking code formatting, and then another ruleset XML that has the extra sniffs (e.g. those labeled with extra in GitHub)

The WordPress-Core ruleset was created in #64 to be a set of "Non-controversial generally-agreed upon WordPress Coding Standards", and in https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/commit/8409c2748e4b95eb6aa9d027e5cf634518886f13#diff-4486463b25768f719431bc763baab473 WordPress-Extra was introduced, as "Best practices beyond core WordPress Coding Standards".

Ever since, the Core ruleset has essentially reflected the standards outlined in the handbook, while Extra contains additional "best practices".

Present Discussion

In #1157 it was proposed to split formatting and non-formatting rules into their own subsets. But in the process of the discussion, there was soon talk of renaming Extra and Core while we were at it.

The main relevant issues raised are:

  1. The name Extra may not be ideal.
  2. The Extra ruleset is just a catch-all for stuff not in Core.
  3. The term Core implicitly ties the standard too closely to a single project, instead of the community standards in the handbook.

Confusion around Extra

One reason this came up is because the Extra ruleset is sometimes confusing.

To be honest, I never understood the Extra ruleset. To me, this always sounded like stuff that doesn't really belong somewhere else, or rather: you are not sure about where it does belong. https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1157#issuecomment-331635949

It was also noted that splitting it would just add confusion (https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1157#issuecomment-331636969). In the end, I suggested that clearing this up first may help to move forward with the rest of #1157. So here we are.

Why is the distinction needed?

This also led to some discussion of why the distinction between Core and Extra really matters. Why can't they just be combined? The answer is essentially because Core is tied to the handbook, and the handbook isn't comprehensive.

The Core ruleset is only supposed to cover that what's literally stated in the WP PHP Coding Standards handbook. Nothing more.

However, on the one hand the handbook is far from comprehensive, there are a lot of areas which are not covered or only implied. On the other hand, there are industry standards and both PHP as well as WP best practices.

Tongue in cheek, I would go as far as saying that Extra currently contains everything we wish would be covered by the handbook, but isn't. https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1157#issuecomment-331639562

Getting stuff into the handbook would partly alleviate this, and was further propounded:

Right now, WordPress-Extra is a more guiding ruleset based on the opinionated experience of a few of the admins here, but it doesn't need to be. For a long while, the flow of implementation has pretty much all been one way - from the Handbook into the WPCS. But, with WP Core now more involved, we have the opportunity to make that two-way, with WordPress-Extra feeding back into the Handbook. https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1157#issuecomment-351834628

It was noted that this is easier said than done though.

Core and Core

Ultimately, it was suggested that the name Core is also problematic, because the handbook is for

when writing PHP code for WordPress, whether for core programming code, plugins, or themes.

Therefore:

I'd rather see the Core bit dropped altogether, since WP Core is just one of many bits of software that is (meant to be) using the standards (which only happen to be documented in the Core Handbook out of convenience to it being anywhere else). https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1157#issuecomment-346306078

And:

Drop the term 'Core', since that is just one project in the WP community which is using these standards. If WordPress Core wants to (temporarily) do something different to what the Handbook says, then it has its own phpcs.xml.dist with which to make the adjustment. Plugin and theme authors should feel that they can follow what the community standards are, not what WP Core is doing at any moment in time. https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1157#issuecomment-331732520

However, the relationship between the handbook and core vs the community as a whole has been less clear in practice.

Analysis

Here is my analysis on the points that need to be addressed here, and my recommendations for how to address them.

The Handbook

WPCS started out covering just a few rules from the standards in the handbook, and has now grown to the point that the handbook is the thing running behind and becoming a bottleneck. This is a measure of success and shows that WPCS has reached an important milestone. ๐ŸŽ‰

Continuing advancement on the part of WPCS is currently being affected by a pessimistic outlook on the further development of the standards in the handbook. We can work around the handbook's lagging, but in the end I think it will cramp WPCS and be less than ideal for the WP community as a whole.

It is time for WPCS and the handbook to decide exactly what they want to be when they grow upโ€”because they have grown up. And yet at this point it still seems a little fuzzy, at least in my opinion.

It is clear that a tighter relationship is forming between WPCS and WordPress itself. However, despite recent advances, I don't think the relationship has fully matured. WPCS still seems, to me at least, more like the de facto standard (pun intended), rather than the "official" WP coding standards for PHPCS. (There is no mention of it in the handbook, for example.) Whether you call it "official" or not, and despite some overlap in leadership and contributors, the fact is that WPCS still seems distinct and autonomous. In particular, the handbook and WPCS are administered very differently; there is no clear process for changing the handbook.

The handbook describes itself as the community standards, but in practice the community doesn't have a clear avenue of offering input at present. If the coding standards are really going to be the community standards, then there needs to be a clear process for discussion and input on the part of the community. Because otherwise the community will go on with WPCS and Extra, and if, when the handbook finally "catches up", it ends up looking different than Extra, the community's ship will probably have already sailed. The result will likely be a disparity between what is in the handbook and what has come to be practiced by the community.

๐Ÿ‘‰ There needs to be a clear process for community contribution to the handbook. ๐Ÿ‘‰ The "official" status of WPCS needs to be made clear.

Extra, Extra, Read all about it

However, this will not erase the line between Core and Extra. As I see it, Extra contains several different classes of sniff:

It is in this sense that it is a catch-all for everything not in Core.

The latter two groups could be absorbed into the handbook. And part of me says that ideally, Extra should go completely. But where I am less certain is some of the QA/best practices. I'd like to think that most of these ought to be official, but I suspect that not every QA check necessarily has universal applicability.

Basically it comes down to a question: Why do we need to provide anything that isn't in the handbook?

At this point though, it is hard to tell.

I've already suggested that we need to fix the handbook's lagging, above. But that still doesn't deal with remaining items which may not belong in the official standards for various reasons. At this point though, I think it is difficult to say exactly what will go in the handbook and what will not, and why. Until the processes around the standards are made clear, the future of Extra will remain murky.

Extra suffers from the fact that it isn't actually a standard. I think things are better when each ruleset has a single purpose, a single reason for existence. (E.g., Core exists to supply the rules outlined in the handbook.) In other words, there should just be one right and clear answer to this question:

Right now, there are multiple possible answers to that question, and none real clear. I think the goal should be to get to a point where that is no longer true.

As a general outline, going forward, I would like to see the more things move from Extra to the handbook. Then, a discussion should happen about whatever remains (if anything) that cannot be moved to the handbook, for whatever reasons. It may then be useful to either, a) remove some of these things from WPCS entirely, and/or b) split Extra into several sub-sets around each reason for keeping things in WPCS that aren't in the handbook. Exactly what happens, though, will depend on what is left. This in turn, will depend on what the standards in the handbook will absorb, and what that process will look like.

But basically, the end goal would be the Extra would either no longer exist, or at least no longer be murky (exist for just a single reason).

๐Ÿ‘‰ We should move as much from Extra to the handbook as possible. ๐Ÿ‘‰ The remaining ruleset (or subsets) should have a single clear purpose.

Summary

WPCS has grown, while activity around the standards in the handbook has perhaps waned. WPCS has eclipsed the handbook, but is still constrained by it. Going forward, changes need to be made so that the energy around WPCS can be channeled into the handbook rather than it acting as a resistor. Essentially, the relationship between WPCS and the handbook and the community and core needs to come to the next stage of maturity. That process is in progress, but on WPCS's end (at lest in my view) things are still fuzzy. WPCS is here to serve the WP community, but how it wants to/is expected to do that going forward has not been precisely pinned down and made explicit.

This is a discussion that I think needs to happen now rather than later, so that we can confidently move forward with the major changes that have been proposed for the rulesets that we offer.

In short, as I said above, WPCS and the standards in the handbook have grown up, and now they need to decide exactly what they want to beโ€”and then be it.

Next Steps

This is a conversation that needs to involve more than just the WPCS contributors. Really, it is more about the WP community and project leads helping to set the direction here and clear the way for further growth. Discussion probably ought to take place in Slack and on the make blogs. The key things that need to be tangibly defined are:

Then, WPCS will be able to proceed within a better defined framework for the future.

1157

I actually don't think this has to be a blocker for #1157. However, I also think that clearing this up will help us to move forward there more confidently. I suggest that if #1157 does proceed before this is dealt with, that it should not rename either the Extra or Core rulesets. This would be with the understanding that we might still rename/restructure Extra and Core, depending on how this issue pans out. And since Extra is mostly best practices rather than codestyle anyway, I'd say for now it should stay that way, and the focus would be on splitting the Core ruleset.

Feedback

I apologize for being introspective and philosophical. If you think this conversation would just be a distraction, feel free to make that case. ๐Ÿ˜„

Otherwise, I guess what we need to hear is:

In my view, ideally, we end up with a standard that is truly a community standard, and not just for core. Core could then be renamed, and Extra would either no longer exist or have a more clearly defined purpose and direction (and could likewise be renamed to better fit that purpose).

And finally, since this is so long, you also have permission in advance to slap me. But not too hard. ๐Ÿ˜„

ntwb commented 6 years ago

Thanks for this @JDGrimes ๐Ÿ‘

Here are my initial thoughts to address some of the above:

๐Ÿ‘‰ There needs to be a clear process for community contribution to the handbook.

Let's swap out the current coding standards handbook pages for a GitHub repo that syncs all of the WordPress Coding Standards and Inline Documentation Standards handbook pages. Replicating the way the REST-API [handbook]()(repo) and WP-CLI handbook(repo) projects are currently performing this using the wporg-markdown plugin to sync markdown from GitHub repos to WordPress.org handbook content.

๐Ÿ‘‰ The "official" status of WPCS needs to be made clear.

Let's update this where required, these are a few pages that immediately come to mind: โ€ข WordPress Coding Standards โ€ข Inline Documentation Standards โ€ข Code Refactoring <- This page should probably be deleted entirely ๐Ÿ˜‰

๐Ÿ‘‰ We should move as much from Extra to the handbook as possible.

RFCs, how about an RFCs repo? https://github.com/WordPress-Coding-Standards/rfcs, inspired by Reacts recent addition of adding an RFCs repo https://github.com/reactjs/rfcs this could be used to help propose/update coding standards.

dingo-d commented 6 years ago

Let's swap out the current coding standards handbook pages for a GitHub repo that syncs all of the WordPress Coding Standards and Inline Documentation Standards handbook pages. Replicating the way the REST-API handbook(repo) and WP-CLI handbook(repo) projects are currently performing this using the wporg-markdown plugin to sync markdown from GitHub repos to WordPress.org handbook content.

When I started reading the thread this is something that came to my mind. Either do it like Gutenberg team is doing (https://wordpress.org/gutenberg/handbook/ is tied to https://github.com/WordPress/gutenberg/tree/master/docs), or try to sync the handbook documentation with the comments already made in the sniffs (something like on https://developer.wordpress.org/reference/functions/ where documentation is made from the docblocks in the WordPress core).

I personally wasn't aware of the inline documentation at all, so this should all be tied to a single coding standards handbook.

grappler commented 6 years ago

Do we agree that we'd like clearer definition around the relationship between WP and WPCS?

It may be a good idea to discuss if this should become an official project like WP-CLI has? Though it is important that this does not introduce more bureaucracy.

Do we agree that we'd like a clearer process around updating the handbook? What kind of process and relationship would we like to see?

The Coding Standards handbook is currently under core which would mean this are the coding standards that applies to core but themes and plugins can use. It made sense to add it there as it applies to all three areas.

If WPCS would become it's own WordPress project then it could have it's own handbook like WP-CLI and REST-API under https://developer.wordpress.org/

The WP Standards should apply to the community. Core can make the necessary changes are needed. How would it be decided which standards are added?

How would you define the purpose of Extra, and do you think this should be clarified/refined in the future?

It depends on if we make a distinction between coding standards and best practices. I agree that Extra can become a bit of a dumping ground.

As Tide will be using WPCS in the future it would be good idea to think how WPCS can be used as a review tool. Meaning thinking how we can seperate things that coding standards and that don't have an influcence on code quality and best practices. It would be nice to have the Tide ruleset added here once it get's so far.

ntwb commented 6 years ago

When I started reading the thread this is something that came to my mind. Either do it like Gutenberg team is doing (https://wordpress.org/gutenberg/handbook/ is tied to https://github.com/WordPress/gutenberg/tree/master/docs),

The Gutenberg docs use the same wporg-markdown plugin, though instead of the standard make.w.org wporg-breathe theme which the majority of Make P2s and Handbooks use it uses it's own Gutenberg theme (it's a w.org specific fork of https://github.com/WordPress/gutenberg-theme)

or try to sync the handbook documentation with the comments already made in the sniffs (something like on https://developer.wordpress.org/reference/functions/ where documentation is made from the docblocks in the WordPress core).

Could be worth investigating, initially, I think it would involve considerable work in updating/forking the WordPress/phpdoc-parser and updating of the PHPDoc's here in WPCS, self-documenting docs for a v2 maybe.

I personally wasn't aware of the inline documentation at all, so this should all be tied to a single coding standards handbook.

JS & PHP have their own pages for inline docs, CSS has it included in its coding standards page.

So sure we could look into what would work the best, a single page for everything per language or break them down further into even smaller pages

ntwb commented 6 years ago

It may be a good idea to discuss if this should become an official project like WP-CLI has? Though it is important that this does not introduce more bureaucracy.

I'd like to think we are already, we don't have our own Make P2, we could if we saw a need for it.

The Coding Standards handbook is currently under core which would mean this are the coding standards that applies to core but themes and plugins can use. It made sense to add it there as it applies to all three areas.

Using the wporg-markdown sync plugin the pages could also be sync'd to the theme and plugin handbooks.

If WPCS would become it's own WordPress project then it could have it's own handbook like WP-CLI and REST-API under https://developer.wordpress.org/

I'm not sure that is entirely required just yet, as I mentioned above we can sync these docs to numerous handbooks and we could quite easily add a new coding standards header section to https://developer.wordpress.org/ linking to appropriate handbooks just as the REST-API and WP-CLI do currently.

The WP Standards should apply to the community. Core can make the necessary changes are needed. How would it be decided which standards are added?

That would be via the RFCs proposal I mentioned in https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1269#issuecomment-354243835

As Tide will be using WPCS in the future it would be good idea to think how WPCS can be used as a review tool. Meaning thinking how we can seperate things that coding standards and that don't have an influcence on code quality and best practices. It would be nice to have the Tide ruleset added here once it get's so far.

Tide's ruleset is currently all of WPCS', i.e. it includes all WordPress-Doc, WordPress-Extras, WordPress-VIP, WordPress-Core WPCS rulesets (Source: Slack #tide channel 20th Dec 2017)

JDGrimes commented 6 years ago

Thanks for the input so far, everyone. I think @ntwb has some great proposals here. I was not aware that the tools for linking a repo to handbook pages already existed. I think that is a good idea. I suppose we'd need to discuss that with the docs team?

An RFC repo is also an interesting idea. I think that would be a better fit than a Make P2 for making suggestions and gathering feedback in regard to things in Extra that could be added to the handbook.

Aside from these implementation details, what steps need to be taken to get the blessing of the community/the powers that be before proceeding with any of this? Should a proposal be brought up in a core meeting? On the core P2? I guess basically what we'd like to do is sort of assemble a "coding standards team," and I'm not sure who all among various existing teams that currently falls under.

ntwb commented 6 years ago

Thanks for the input so far, everyone. I think @ntwb has some great proposals here. I was not aware that the tools for linking a repo to handbook pages already existed. I think that is a good idea. I suppose we'd need to discuss that with the docs team?

Nope, we're fine to kick this off ourselves, each team is responsible for their own handbooks, the docs team covers the codex, the HelpHub project, and general documentation that isn't specific to any other team.

I'm thinking a repo here https://github.com/WordPress-Coding-Standards/docs would do the trick, I'd initially thought /handbook though using /docs I think gives us a little more flexibility of what the repo can be used for.

An RFC repo is also an interesting idea. I think that would be a better fit than a Make P2 for making suggestions and gathering feedback in regard to things in Extra that could be added to the handbook.

There are some pending decisions to be made in regard to JavaScript Coding Standards and I've not yet raised but will be proposing some CSS and SCSS changes in the near future, a single RFC repo can cover all of PHP, JS, and CSS coding standards is what I'm envisaging.

Aside from these implementation details, what steps need to be taken to get the blessing of the community/the powers that be before proceeding with any of this? Should a proposal be brought up in a core meeting? On the core P2? I guess basically what we'd like to do is sort of assemble a "coding standards team," and I'm not sure who all among various existing teams that currently falls under.

Yeah, raising it in a #core meeting would be great to both garner further interest and raise awareness of what we are proposing

Pinging @pento for some feedback and thoughts on the above

tl;dr: Create a new repo here for the Coding Standards handbooks under VCS to sync back to the existing make/core handbook pages. Add another repo, an RFC repo for discussing and proposing changes to the coding standards.

JDGrimes commented 6 years ago

Just a heads up for everyone: turns out my life is going to be a little crazy for the next few months, so I'm probably not going to be able to help spearhead this as I would have liked to. (And I don't know yet how involved I will be after that.)

So don't wait for me and my input on stuffโ€”whatever is done here is probably going to have to be done without me, or at least without me helping to spearhead it. โ˜น๏ธ

pento commented 6 years ago

I'm not wild about adding yet another docs repo, they're annoying to setup and maintain, particularly in the current ad-hoc manner. We're running a risk of fragmenting the docs again, even before helphub is finished. Why isn't editing the handbook pages directly an option?

I'm fine with RFCs, in the style of React's RFC repo - it's a place to propose changes that may or may not be accepted by the core team, not a place to vote brigade.

GaryJones commented 6 years ago

Why isn't editing the handbook pages directly an option?

Right now, of those currently active here, only you and @ntwb appear to have Handbook editing capabilities.

it's a place to propose changes that may or may not be accepted by the core team

That highlights one of the concerns that this ticket is about. The standards are for everyone in the WP community - not just Core. It would be easy for "the core team" to veto not adding standards to the Handbook that cover how namespaces, traits, and return types (etc.) should be formatted because they aren't used by Core - this is exactly what's happened before.

If we do go the RFC route (my only experience is seeing how php-core does it), then we'd need a list of named folks who could vote. Maybe it's some or all core folks, as well the WPCS maintainers, but I'd hope that all of the folks on the list would have an interest and understanding of CS.

pento commented 6 years ago

I'm fine with adding more people to be able to edit the Handbook.

I'm also okay with the Core coding standards including things from newer version of PHP, though we'd obviously need to flag what's currently allowed for Core, and what will be allowed as we upgrade minimum PHP version. The goal of the servehappy team is to allow Core to drop older PHPs, and there are a few other efforts to push that forward, too - we should be expecting that anything defined in the coding standards will ultimately be adopted by Core.

Finally, -1000 on a php-core style RFC process. Everything about it reeks of drama and personality politics, I don't need that in my life. The process I think would work is something like:

  1. Proposal is submitted on the WPCS RFC repo.
  2. WPCS folks give feedback, iterates, etc. Drag in Core folks as needed.
  3. Once WPCS folks are happy with it, ping Core folks and lead devs for any other feedback, final approval.
  4. Proposal is adopted, Handbook is updated, standard is applied to Core (if needed).

I trust y'all to have good sense on what will work for Core and the wider community, including proposals that can't be adopted by Core yet, but will be in the future.

ntwb commented 6 years ago

FYI: HelpHub is user oriented docs, not developer docs:

Via https://github.com/kenshino/helphub "The WordPress user documentation portal https://make.wordpress.org/docs/category/helphub"

On the handbook sync thing, one of my thoughts above was in regard to the rise of Tide as theme and plugins would be advised to also follow core coding standards, as such I was thinking a canonical source of the coding standards sync'd to the core, theme, and plugin handbooks.

pento commented 6 years ago

Oh, right. Yeah, I meant developer.wordpress.org, not helphub.

It seems like this would ultimately live in developer.wordpress.org, as a resource for anyone building stuff for WordPress.

ntwb commented 6 years ago

I wrote above https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1269#issuecomment-354406841

If WPCS would become it's own WordPress project then it could have it's own handbook like WP-CLI and REST-API under https://developer.wordpress.org/

I'm not sure that is entirely required just yet, as I mentioned above we can sync these docs to numerous handbooks and we could quite easily add a new coding standards header section to https://developer.wordpress.org/ linking to appropriate handbooks just as the REST-API and WP-CLI do currently.

So, maybe we go for adding a new section on https://developer.wordpress.org now then?

e.g. https://developer.wordpress.org/coding-standards ?

pento commented 6 years ago

Yah, I can see that being a good option. If the goal is to have a global coding standard, it would need to be linked from the Plugin and Theme handbooks, too.

It's also way easier to remember the URL than somewhere deep in the make/Core handbook, too. ๐Ÿ˜‰

ntwb commented 6 years ago

It's also way easier to remember the URL than somewhere deep in the make/Core handbook, too.

Done, I'll work in that direction and add meta tickets as needed

philipjohn commented 6 years ago

Going back to @JDGrimes' question of structuring the rulesets

This also led to some discussion of why the distinction between Core and Extra really matters

and @pento's comment

I'm also okay with the Core coding standards including things from newer version of PHP, though we'd obviously need to flag what's currently allowed for Core,

One solution might be too split the rulesets into;

Hopefully that'd help answer @grappler's question:

The WP Standards should apply to the community. Core can make the necessary changes are needed. How would it be decided which standards are added?

The Core ruleset would effectively be subject to veto by the core team because it would only apply to core development, with their input sought on the Community ruleset. Likewise, VIP would maintain our ruleset but with input and guidance from everyone here.

GaryJones commented 6 years ago

I'm fine with adding more people to be able to edit the Handbook.

@pento I'd be happy to have edit rights.

The process I think would work is something like:

  1. Proposal is submitted on the WPCS RFC repo.
  2. WPCS folks give feedback, iterates, etc. Drag in Core folks as needed.
  3. Once WPCS folks are happy with it, ping Core folks and lead devs for any other feedback, final approval.
  4. Proposal is adopted, Handbook is updated, standard is applied to Core (if needed).

That's pretty much what we've been doing on an ad hoc basis until now. What would be good, would be to have a documented list of "Core folks" from the PHP, JS and CSS side who would be interested enough and able to chip in as needed.

e.g. https://developer.wordpress.org/coding-standards

+1 for that. That then solves what @philipjohn mentions; Community standards (PHP, JS and CSS) would be documented at developer.wordpress.org/coding-standards, but Core standards (i.e. what they can manage bearing in mind the PHP restrictions, for instance) can live where it does now - basically referencing the developer.wordpress.org page, but with the following exceptions.

How would developer.wordpress.org/coding-standards be generated? Is that manual / standard PHP files, or some build script from Markdown etc.? Is it under version control somewhere (Meta?)

ntwb commented 6 years ago

For the DevHub part of this, I've created the repo and imported the existing handbook pages:

https://github.com/WordPress-Coding-Standards/docs

I'll create issues on that repo to documented what other tasks are required.

ntwb commented 6 years ago

The RFCs repo is now published https://github.com/WordPress-Coding-Standards/rfcs

There's 3 proposed coding standards changes that should arrive in that repo soon ๐Ÿ˜„