? Success

User tests: Successful: Unsuccessful:

avatar wilsonge
wilsonge
24 Aug 2014

Code style. (Whitespace.ContatenationSpacing)

avatar wilsonge wilsonge - open - 24 Aug 2014
avatar jissues-bot jissues-bot - change - 24 Aug 2014
Status Pending New
Labels Added: ?
6bbb74c 24 Aug 2014 avatar wilsonge Caps
avatar brianteeman
brianteeman - comment - 24 Aug 2014

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 ' .

avatar pjwiseman pjwiseman - change - 24 Aug 2014
The description was changed
Status New Ready to Commit
avatar pjwiseman pjwiseman - change - 24 Aug 2014
The description was changed
avatar wilsonge
wilsonge - comment - 24 Aug 2014

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!

avatar brianteeman
brianteeman - comment - 24 Aug 2014

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 :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!


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

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar pjwiseman
pjwiseman - comment - 24 Aug 2014

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

  • <!-- We only want this for libraries and cli for now -->
  • /administrator/*
  • /components/*
  • /plugins/*
  • /modules/*
  • /installation/*
  • /templates/*
  • /language/*
  • /includes/*


  • This comment was created with the J!Tracker Application at http://issues.joomla.org/.
avatar pjwiseman
pjwiseman - comment - 24 Aug 2014

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/.

avatar mbabker
mbabker - comment - 24 Aug 2014

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.

avatar pjwiseman
pjwiseman - comment - 24 Aug 2014

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/.

avatar Bakual Bakual - change - 25 Aug 2014
Status Ready to Commit New
Labels Added: ?
avatar dbhurley dbhurley - change - 25 Aug 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-08-25 14:44:19
avatar dbhurley dbhurley - close - 25 Aug 2014
avatar dbhurley dbhurley - close - 25 Aug 2014
avatar wilsonge wilsonge - head_ref_deleted - 25 Aug 2014
avatar wilsonge
wilsonge - comment - 25 Aug 2014

Add a Comment

Login with GitHub to post a comment