Update 4 letters for min prefix length

[davidperezgar]

Fixes #2467

[dingo-d]

I've added some comments about the docblock changes.

This change should also be covered by tests (warning on prefixes less than 4 characters).

[jrfnl]

Thanks for PR @davidperezgar.

Can you please do things properly though ? It looks like you don't understand how docs work.

• The @since tag should NOT be changed as it indicated when the constant was added. A second @since tag can be added to in explain that the value of the constant changed in WPCS 3.2.0.
• The @link tag should NOT be changed as it is a reference to explain why the constant was added. A secondary @link tag can be added to the new ticket to explain why it was raised.

Also, this is a functional change, so the tests will need to be updated to match, as can also be seen by the failing build.

[jrfnl]

Ha, looks like @dingo-d and me were reviewing at the same time, but had the same feedback ;-)

[davidperezgar]

Ok! I'll check it , thanks.

[jrfnl]

Still missing a test with a three character prefix which should be flagged as not acceptable.

[davidperezgar]

I don't understand how it works the unit testing in the sniff.. I'd need some help...

[jrfnl]

I don't understand how it works the unit testing in the sniff.. I'd need some help...

Did you read the CONTRIBUTING guide ?

[davidperezgar]

Did you read the CONTRIBUTING guide ?

Yes!

[GaryJones]

I see that this sniff is included in WordPress-Extra.

I also see the history of this change is related to plugins, but what about themes and the theme review team? This change will affect those folks who are writing themes with custom functions too - has that impact been considered?

[davidperezgar]

Hello, yes it will impact to themes as well, but we have been considered the benefits of this measure, and that's why is now in the Developers HandBook:
https://developer.wordpress.org/plugins/plugin-basics/best-practices/

We were using this number of prefixes for a some time ago and we are unifying everywhere.

I'm going to ping Themes Team to be aware of it.

[GaryJones]

There are several mentions of "prefix" in the theme developers handbook, but:

• https://developer.wordpress.org/themes/getting-started/reading-this-handbook/ and
• https://developer.wordpress.org/themes/basics/theme-functions/

... seem to be the missed opportunities to clarify/define the specific minimum length of such prefixes.

It seems right (to me) to get their buy-in on this before we merge this PR; the Plugins Review Team can continue to require 4 characters as they have apparently have for a while anyway).

[davidperezgar]

I've spoken with Team Rep and they will check with the Team. I don't see a problem for that. Thanks @GaryJones

[kafleg]

@GaryJones Thank you for sharing the link. Once we decided to have 4 letter for min prefix length, we'll discuss within the team for updating the handbook.

[davidperezgar]

Thanks! @kafleg !

[rodrigoprimo]

I believe the sniff documentation needs to be updated as well to reflect the changes proposed in this PR:

https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/WordPress/Docs/NamingConventions/PrefixAllGlobalsStandard.xml#L102-L118

[davidperezgar]

I believe now is ready to be reviewed. Thanks.

[rodrigoprimo]

Thanks for implementing the changes, @davidperezgar! Looks good to me now.

@WordPress/wpcs, as far as I can check, all the points raised were addressed and this PR is ready to be merged.

Edit: @jrfnl (as we discussed this via DM), it seems that mentioning teams is not working in this repository. Maybe due to the way permissions are configured in this GH organization. Mentioning the other maintainers as well directly as the team mention above didn't work: @GaryJones and @dingo-d.

[rodrigoprimo]

@davidperezgar and @kafleg, any updates on including the minimum four-letter prefix rule in the theme handbook? @GaryJones mentioned he'd prefer this information to be added to the theme handbook before merging this PR.