There doesn't seem to be a rule for detecting variables in this style, such as $header_background_color (instead of the MediaWiki convention, $headerBackgroundColor). Is this a sniff that could be considered under the scope of MediaWiki PHPCS? Is this instead detected by our MediaWiki Phan configuration?
Description
Description
Event Timeline
Comment Actions
I haven't written a sniff for PHPCS but would be interested in contributing anyways, but I'm fine if someone else would want to take over (since I am also a bit busy).
Comment Actions
May I ask what the problem with variable names like $background_color is? I mean, other than that a code style mentions a different naming scheme?
As far as I'm concerned, PHPCS sniffs are about readability. Code should not be surprising but quick and easy to understand. Ok. But how is $background_color hard to read?
I don't know how accurate the numbers are Codesearch reports. But it looks like there are thousands of files: https://codesearch.wmcloud.org/search/?q=%5C%24%5Ba-z%5D%2B_%5Ba-z%5D&files=%5C.php%24. More than 200 matches in core alone: https://codesearch.wmcloud.org/search/?q=%5C%24%5Ba-z%5D%2B_%5Ba-z%5D&files=%5C.php%24&repos=MediaWiki%20core. Who is going to fix these? Or to ask the question a bit different: Are we sure the resources needed to do this are worth it? I.e. does the code really become so much more readable and easier to maintain that it's worth investing the time?
Comment Actions
Per https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP#Naming, the convention for mediawiki code is to use lowerCamelCase
As far as I'm concerned, PHPCS sniffs are about readability. Code should not be surprising but quick and easy to understand. Ok. But how is $background_color hard to read?
Its not necessarily hard to read, but then again neither is having an extra space where we conventionally don't have one, or missing a space where we conventionally include one
I don't know how accurate the numbers are Codesearch reports. But it looks like there are thousands of files: https://codesearch.wmcloud.org/search/?q=%5C%24%5Ba-z%5D%2B_%5Ba-z%5D&files=%5C.php%24. More than 200 matches in core alone: https://codesearch.wmcloud.org/search/?q=%5C%24%5Ba-z%5D%2B_%5Ba-z%5D&files=%5C.php%24&repos=MediaWiki%20core. Who is going to fix these? Or to ask the question a bit different: Are we sure the resources needed to do this are worth it? I.e. does the code really become so much more readable and easier to maintain that it's worth investing the time?
It isn't a big issue if the sniff gets suppressed, and fixes those variable names is a fairly simple task that, though probably shouldn't be done by phpcs automatically, can easily be done by new contributors (eg a good first task for developers just starting out) and reviewed without complications. Additionally, by having such a sniff we can avoid introducing more such variables, which are discouraged by our coding conventions
Comment Actions
[…] the convention for mediawiki code is to use lowerCamelCase
I know, and I prefer this convention myself. But this is not really an answer to my original question. What is the issue with $foo_bar other than not being camelCase?
[…] can easily be done by new contributors (eg a good first task for developers just starting out) and reviewed without complications.
With all the respect, but I don't think new contributors can learn anything from replacing thousands of underscores.
Comment Actions
@thiemowmde Thank you for your input! There's not really any issue besides being consistent with the coding conventions.
However it would be hypocritical for me to say this, as my own extension (StructuredNavigation, that both of you have seen by now) doesn't follow the Wikimedia coding conventions for PHP docblocks (although in my opinion follows pretty much everything else).
I will back off from this task for now, as it's clear it may not be useful at this time.