User tests: Successful: Unsuccessful:
Code style. (Whitespace.ContatenationSpacing)
Status | Pending | ⇒ | New |
Labels |
Added:
?
|
Status | New | ⇒ | Ready to Commit |
Doesn't look like it's documented. I also like how the rules also have a '.' without a space in the sql example :P
But when we run the tests it comes up http://mbabker.github.io/jcms-codestyle/reports/modules.xml as "Concat operator must be preceeded by one space" which is this file here https://github.com/joomla/joomla-cms/blob/staging/build/phpcs/Joomla/Sniffs/WhiteSpace/ConcatenationSpacingSniff.php#L18
It's one of the biggest breakers of the code style rules, so it doesn't suprise me there are 2.9k uses of it!
Would you be able to document it ;)
On 24 August 2014 22:57, George Wilson notifications@github.com wrote:
Doesn't look like it's documented. I also like how the rules also have a
'.' without a space in the sql example :PBut when we run the tests it comes up
http://mbabker.github.io/jcms-codestyle/reports/modules.xml as "Concat
operator must be preceeded by one space" which is this file here
https://github.com/joomla/joomla-cms/blob/staging/build/phpcs/Joomla/Sniffs/WhiteSpace/ConcatenationSpacingSniff.php#L18It's one of the biggest breakers of the code style rules, so it doesn't
suprise me there are 2.9k uses of it!—
Reply to this email directly or view it on GitHub
#4172 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
This code styling was added by the following commit, though only required for libraries and cli. I wonder why it's now being applied to modules as there have been no subsequent commits. I'm assuming mbabkers code styling reports referenced above are an attempt to be more extensive, but perhaps only use by Michael for code he is committing? Is there a reference to why you're using these code styling reports. Regardless, I think it's a good idea. Code reviewed successfully.
commit 7c3aa93
Author: Rouven Weßling me@rouvenwessling.de 2012-09-10 01:47:55
Committer: Rouven Weßling me@rouvenwessling.de 2012-09-10 01:47:55
I'm assuming 1 test is sufficient for this one. Moving to RTC.
This comment was created with the J!Tracker Application at http://issues.joomla.org/.
The code style report in that repo has basically removed most of the exemptions that are in place in the current code style checks as a guide to help those who are interested in getting the CMS code as compliant with the standard as possible.
If we took all the exceptions off, our CI reports would be unusable as there's still hundreds of errors in the code base that are hidden because of the current config.
Thanks Michael - makes sense.
@wilsonge I've raised a PR#74 for the code standard problem you spotted in the SQL example.
This comment was created with the J!Tracker Application at http://issues.joomla.org/.
Status | Ready to Commit | ⇒ | New |
Labels |
Added:
?
|
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-08-25 14:44:19 |
@brianteeman Added a quick note at joomla/coding-standards#75
Where is this codestyle documented for apostrophe space period - I looked here http://joomla.github.io/coding-standards/ and didnt see it but when I search the codebase I can see 2900 instance of '. and 10000 of ' .