[#13022] - Coding standards - References and control structures
- Fixed in Code Base
- 6 Dec 2016
- Medium
- Build: staging
- # 13022
- Diff
- marcochirienti:staging
- Success continuous-integration/drone the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Summary of Changes
Fixed coding style:
- for all control structures there is a space between the keyword and an opening parenthesis;
- when using references, there should be a space before the reference operator and no space between it and the function or variable name.
Testing Instructions
Code review
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_installer com_modules com_plugins com_tags com_templates com_users Templates (admin) Front End com_contact com_content com_finder com_newsfeeds Installation Layout Templates (site) |
| Labels |
Removed:
?
|
||
| Title |
|
||||||
| Title |
|
||||||
I was thinking to proceed in a step by step approach, to make code review easier and to prevent accidental mistakes.
I found 361 hits in 155 files inside the branch for ):, so i was planning to make a separate pull request for this.
There are also other things that i think should be reviewed in these files, e.g. in control structures the opening curly bracket should be on a new line.
If you prefer i can make all the changes and then update the pull request.
Please, forgive my poor English, but i'm not a native speaker.
If you prefer i can make all the changes and then update the pull request.
ok. there's really no need if it's merged faster.
i was just trying to avoid PR conflicts because PR sometimes take some time to merge.
Please, forgive my poor English, but i'm not a native speaker.
me neither
@zero-24 please check this one also, so it's merged faster to avoid conflicts if decided
I have tested this item
on code review
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13022.
I have tested this item
Code review
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13022.
@marcochirienti one advice, if/after this PR is merged, make a specific branch for each PR you make and keep your staging branch sincronized with joomla main staging branch or you will get trouble in the future.
[...] or you will get trouble in the future.
That sounds like a threat, @andrepereiradasilva
Thank you for the advice, Sir.
That sounds like a threat, @andrepereiradasilva
😄
lol, just tryng to make your life easier ....
Thank you, i appreciate it.
As i said, i'm completely new to all this stuff, so any advice is welcome.
We appreciate you spending the time doing this
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13022.
| Milestone |
Added: |
||
@marcochirienti Since you are interested in improving the compliance with the code standards I would like to ask you to participate in testing and verifying our updated version of the code standards running on PHPCS 2.x . The updated version of the code standards brings in automated code fixers to help with code style compliance. You can find the WIP PR in the code style repo at this link joomla/coding-standards#109
I'd be glad to do it. I will take a look at it as soon as possible.
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-12-06 22:26:22 |
| Closed_By | ⇒ | rdeutz |
Please add also a space between
):so it becames) :