New WPCS rule (ValidVariableName.NotSnakeCase) causing build error

[colin-marshall]

New rule was added to WPCS that causes build errors. Here is the log:

FILE: ...dpress/src/wp-content/themes/FoundationPress/library/cleanup.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 247 | ERROR | Variable "Foundationpress_img_rebuilder" is not in valid
     |       | snake_case format
     |       | (WordPress.NamingConventions.ValidVariableName.NotSnakeCase)
----------------------------------------------------------------------


FILE: ...hemes/FoundationPress/library/protocol-relative-theme-assets.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 90 | ERROR | Variable "Foundationpress_protocol_relative_theme_assets"
    |       | is not in valid snake_case format
    |       | (WordPress.NamingConventions.ValidVariableName.NotSnakeCase)
----------------------------------------------------------------------

It wants the first letter to be lowercase.

[joshrathke]

Hmmm. That's interesting.

There are actually A TON of checks that we omit in the codesniffer.xml file. Perhaps we should consider not omitting any checks. I mean if it's a WordPress standard. I don't see why we would want to go around certain rules.

I think the only reason was at the time we implemented the checks the theme we were using as reference also omitted those tests.

[colin-marshall]

@josh-rathke unless there is a good reason for the WordPress specific excludes in codesniffer.xml then we should probably remove them. I'll play around with them one of these days and see what kind of errors we get.

In the meantime we should either add the WordPress.NamingConventions.ValidVariableName.NotSnakeCase rule to the excludes or make the changes to prevent build errors.

@olefredrik please give your opinion when you get a chance. Thanks!

[joshrathke]

I would think that upon install composer should be pulling down the latest
build of the sniffs for standards. So I don't know if we should ever have
to "update" the sniffs, other than updating the WPCS build entirely.

But I agree, I think we should lose any exceptions and go entirely WP
standard.

On Wednesday, December 23, 2015, Colin Marshall notifications@github.com
wrote:

@josh-rathke https://github.com/josh-rathke unless there is a good
reason for the WordPress specific excludes in codesniffer.xml then we
should probably remove them. I'll play around with them one of these days
and see what kind of errors we get.

In the meantime we should either add the
WordPress.NamingConventions.ValidVariableName.NotSnakeCase rule to the
excludes or make the changes to prevent build errors.

@olefredrik https://github.com/olefredrik please give your opinion when
you get a chance. Thanks!

—
Reply to this email directly or view it on GitHub
#643 (comment)
.

Josh Rathke
I.T. Director & Web Developer, YWAM Montana | Lakeside
p: (406) 844-2221 m: (765) 637-5847 a: 501 Blacktail Rd. Lakeside MT, 59922
s: www.ywammontana.org e: josh.rathke@ywammontana.org

[Aetles]

The codesniffer rules were more strict in the beginning when @josh-rathke added them in 7acc16c as part of make FoundationPress follow WP Coding Standards but then excludes were added because they were deemed "non-sense", like f4c3c95 and 43033bb. Some WordPress specific excludes where added as well, but without further explanation in 47d28f2. Then even more in 7c2d673 and 24c5fcb.

I can't find it right now but I remembered @olefredrik saying he did not see the need for too strict rules.

[olefredrik]

Hi guys! I think it makes sense to follow the standards 100%, if possible and if it provides value for developers. Since we introduced WP Coding Standards in this project, I've added some new exclude statements in project rule set. The maintainers of the WP Coding Standards repo have on a few occasions added new rules. One of these was relevant only for VIP sites hosted on the wordpress.com domain. It goes without saying that these rules are not relevant for us. Other rules have been excluded because they have been a pain to fix (automatic fix of build errors on localhost has not been possible until recently) and because they have not had a great impact on the quality of the application.

I see that certain rules are excluded in the sample file for WP Coding Standards.

We may want a middle ground though. The best way to do this is add the entire ruleset, then rule by rule, remove ones that don't suit a project.

A review of the rule set would have been useful. If any of you have time to have a look at it, please go for it :). If not, no worries, I'll fix it after the holidays.

Merry Christmas! :)