Allow Class Filenames to Omit class- Prefix

[ggwicz]

Closes #64

Future work could be done to better sniff class filenames (and other kinds of filenames). But for now, this is a great simple tweak to stop flagging class filenames as discussed in issue #64.

[aubreypwd]

The rule here does also eliminate other rules that may not be explicit enough. @ggwicz Can you specify any other rules this omits so we can have FED look at it?

[ggwicz]

@aubreypwd Sure thing. The sniffs are found in whole in this file in the WPCS, and can be summarized as follows:

The class- Prefix Sniff

• This requires all class files,
• In all code (plugins, themes, core, etc.),
• To be named class-{class name}.php

All-Lowercase Class File Names

• This requires all class files,
• In all code (plugins, themes, etc.),
• To be entirely lowercase, and to use hyphens as separators

The -template Postfix Sniff

• This requires non-class files,
• In WP Core's /wp-includes directory only,
• That use template tags,
• To be named {template name}-template.php

☝️ The PR here removes all of these sniffs.

[aubreypwd]

• Please document this change in the README

Here is a good example of how to log it https://github.com/WebDevStudios/WDS-Coding-Standards#110 just make sure you link to this PR so it can be referenced.

[aubreypwd]

@coreymcollins Can you weight in on the -template part of #65 (comment)

@aaroneaton @blobaugh As the moderators, what is your guys take on implementing this? I want Corey to weigh in first, but once he gives his take on it, we need a decision.

[aubreypwd]

On our chat today (bee call) we found a consensus to do this in terms of a structure that is obviously OOP where we can use a custom sniff to find a composer.json file that can flag the folder structure as using PSR4. So this would only affect folders where composer.json says it's PSR4 but leave the class- rule everywhere else.

[aubreypwd]

• @aubreypwd to make this conditional on PSR4 directories

[ggwicz]

@aubreypwd I've documented the change in the readme, so that part is complete now.

As for the todo you left yourself on May 29, I don't think applying this to only PSR-4 directories was championed on scrum—or even if it was, I'd personally vote against that. It just doesn't seem like the right move or a good use of resources. If I recall correctly, this was for a couple of reasons:

• First, that decision is based on treating "uses PSR-4 autoloading" and "uses a more OOP structure" as synonymous, which is not true. OOP is a structure we're striving for everywhere—themes, plugins, client servers running PHP7.x, those running PHP5.6.x, etc.
• Next, there was concern over the fact that projects often have several composer.json files. It seems that creating a sniff that effectively and always grabs the right file is nontrivial. Is the amount of energy tinkering, testing, and tweaking required for that necessary...?

I vote that we take the "quick win" here, and just remove the class- prefix and -template suffix sniff for now. If the class- prefix or -template suffix is desired on a specific project (do any real and current examples exist?), the devs can manually maintain that.

If a really strong desire remains for limiting the scope of this sniff, then I would propose a simpler sniff for that: Instead of looking for PSR-4 info in composer.json files, how about just removing the sniff for files that live in src/ directories?

This way, no clever composer-sniffing has to be done. And in essentially all cases from our work, the types of PHP class files we're talking about exist in a /src directory.

I think this would be easier to implement and easier to maintain.

My personal preference is for simply removing the sniff, of course. But again, if devs want this to be more limited in scope I hope the approach of targeting /src is a good alternative to consider!

[aubreypwd]

As for the todo you left yourself on May 29, I don't think applying this to only PSR-4 directories was championed on scrum

I believe it was, actually (with the information that we can design custom sniffs that can detect this sort of thing).

First, that decision is based on treating "uses PSR-4 autoloading" and "uses a more OOP structure" as synonymous, which is not true. OOP is a structure we're striving for everywhere—themes, plugins, client servers running PHP7.x, those running PHP5.6.x, etc.

True, OOPS is the way of the future, but this structure needs to be only applied to OOP types of structures, especially considering older projects that we currently maintain.

Next, there was concern over the fact that projects often have several composer.json files

There should be a way to channel down to the right composer.json file.

I vote that we take the "quick win" here, and just remove the class- prefix and -template suffix sniff for now. If the class- prefix or -template suffix is desired on a specific project (do any real and current examples exist?), the devs can manually maintain that

I vote against this, there is no reason we can't work to make this work in the right cases, in the right context, at the right time.

how about just removing the sniff for files that live in src/ directories? ... This way, no clever composer-sniffing has to be done. And in essentially all cases from our work, the types of PHP class files we're talking about exist in a /src directory

I agree, this is a way of doing it. But, again, I think this is hasty and there is a correct and appropriate method to approach this vs. a hasty one.

My personal preference is for simply removing the sniff, of course. But again, if devs want this to be more limited in scope I hope the approach of targeting /src is a good alternative to consider!

It is something to consider, but again, as I have always been saying there are ways for us to make sure this works appropriately. If this works, then we could go for it, but if there are cases where this wouldn't work, again, there is no reason we can't build logic to apply this more accurately and appropriately to OOP styled structures.

[ggwicz]

@aubreypwd Cool, thanks for entertaining these changes and laying out your points. The decision needs to be made about which of the following courses of action to take:

1. Do nothing.
2. Merge this PR and remove the sniff.
3. Close this PR for the sake of some future work to make an OOPS-structure sniff for composer files, etc., in which the sniff is only conditionally removed.

How is this decided? Do we just do a vote on a BEE scrum? Or in the Slack channel or something so that folks who don't make the scrum are included...?

Or are you essentially the Czar of the repo and just veto/decide on your own? 😅

Whichever the case, I'd love to see this move forward to a conclusion—let us know what's next! Great chats regardless of the outcome 🙂

[gregrickaby]

@ggwicz

👋 Just wanted to jump in and clear up the decision making process.

BTW: this is going to be a topic on the next Monthly Engineering Scrum on September 4. I want to be as transparent as possible so everyone in engineering is aware of our processes surrounding internal products.

Intro

My job is to empower the engineers at WDS to make decisions. I love it when a new idea, process, or product is proposed. These things are what keep WDS moving forward. bIdeally, when something is proposed, there would be an internal discussion/debate so a roadmap can be written. Once there's a roadmap, someone can pick up the torch and run with it.

With that said, let me draw back the curtain on our existing internal products, which have been born out these debates. Each internal product has a Product Owner (or Gatekeeper...they mean the same thing) and they are as follows:

Internal Products

• WDS Coding Standards (Aubrey)
• Migrations Framework (Jeremy)
• wd_s (Corey)
• CPTUI (Michael)
• WDS SSO (Justin)
• WDS Search w/ Algolia (Me, for now)
• WDS web properties (Me)

Scheduling

The Product Owners are supposed to be forecasted ~4 hours a week to manage these projects. However, given how busy we are, they don't always get time. So tickets can move slower than we'd like, but that's the nature of being in client services. I would ask that everyone, please be patient when this happens.

Stand-ups

I hold a bi-weekly Product Team stand-up, where each Product Owner gives me an update on their product. I try to help guide decisions, build roadmaps, and support these individuals in their efforts.

Final Decision Making

There are times though, when we cannot reach a collaborative decision. As much as we try, people have strong opinions (and that's OK!), but like Spock said, "the needs of many, outweigh the needs of the few" 🖖 and "tie breaking" decision needs to happen. That responsibility falls to me.

It's unfortunate when we can't reach a decision as a group. But if we've exhausted all options and it comes down to opinions vs. facts? We need to make the best decision possible, and keep moving forward.

To answer your questions from above

Aubrey has said on multiple occasions, that ideas for WDS Coding Standards could start on a BEE/FEE/Monthly scrum, but the debate needs to happen in Github issues. That way, everyone will have a chance to weight in, which is great!

And finally, @aubreypwd thinks he's in charge here... because he is. If he makes a decision after carefully weighing the information presented to him? Then everyone needs to be on board with that decision and help support his efforts to push it through. If we don't, we'll never be able to move on to the next idea, process, or product. Which isn't good for anyone.

Thanks for caring enough to have a debate on Coding Standards. You're doing a great job stepping up lately. It's really appreciated. 👍

[aubreypwd]

Just to be clear here, the decision here is to:

1. Just erase the sniffs mentioned here entirely across-the-board in all contexts including themes, plugins, everything (what this PR will do when merged in)

Please correct me if I'm wrong about that.

or

2. Remove these sniffs (through a custom sniff instead, which will require another PR) only in the context in which it applies, e.g. OOP structures like src/, etc where class names do not have class- on them

My vote is (and has been in our BEE call debates) for #2 and some on our BEE call have voiced an opinion for #1 with the justification that it really isn't needed or desired by "anyone" anyways. Which is a valid point, but I tend to approach this justification with caution as this has only been expressed on the BEE call. We have not really been involving FEE's at all or considered other context's like legacy projects, plugins (that we work on) that are on WordPress.org that might have to follow these standards, etc. I really think the concerns here warrants #2 as well-as we can get the same desired affect, w/out having to worry about all the different concerns of removing it globally.

Which weighs into my second point is that we don't have to do it this way. Because we have the option of exploring #2 we can have the same affect by keeping the standard in contexts (like themes which aren't entirely OOP, etc) that use this standard, and exclude it only in contexts where this does not fit (e.g. true OOP structures, etc).

But that is my opinion and my bias....

---

Re: My Bias

The reason I have not been on top of this ticket lately is because I am currently in the process of trying to get https://github.com/aubreypwd/projects/issues/20 done—that allows us to move our current coding standards into a more modern package format. Although it may seem I am ignoring this issue or don't want to push it forward because of my bias, I simply have been putting my time towards finalizing https://github.com/aubreypwd/projects/issues/20.

This issue will likely not get dealt with until we move into that new package-based structure anyways, just to be clear, as my focus has been entirely there for the time being. Once https://github.com/WebDevStudios/php-coding-standards is ready, we may continue this discussion there and work to resolve it with Greg's help if need-be.

But, no matter what gets decided on issues like these, I want everyone to know that I whole-heartedly am listening to everyone's concerns regarding this issue and others like it. And although I will want to defend my own opinion, having Greg be the tiebreaker (before it was supposed to be Aaron and Ben) removes my bias from the decision making process as the lead of this project if need-be. Hopefully Greg's involvement in that step will help quell any confusion around my own bias being a force in the decisions made around debates regarding our coding standards.

Again, I implore everyone, please discuss and document your opinions and facts in Github tickets. Your feedback is what will keep our coding standards moving forward and improving with our new technologies and processes.

[aubreypwd]

I don't think we are going to go with the method (removing the rule) method here. But see WebDevStudios/php-coding-standards#12 on re-working this to be specific for other specifications, e.g. src/, etc.