? ? Success

User tests: Successful: Unsuccessful:

avatar shopfe
shopfe
18 Mar 2014
avatar shopfe shopfe - open - 18 Mar 2014
avatar shopfe
shopfe - comment - 18 Mar 2014

i will fix this

avatar betweenbrain
betweenbrain - comment - 18 Mar 2014

Thank yoiu for your contribution! Unfortunately, this PR may be difficult to merge as there are many files being modified, each with a different code style issue. Some of the changes are very good, but there's are others that mat delay or prevent this PR from being accepted. I would recommend trying to break this into smaller pull requests, each limited in scope to one code style fix. That will make it much easier to test and merge. Thanks again!

avatar betweenbrain
betweenbrain - comment - 18 Mar 2014

You may also want to take a look at http://joomla.github.io/coding-standards/ to verify some of the style changes.

avatar shopfe
shopfe - comment - 18 Mar 2014

Hmm,

i used https://github.com/joomla/coding-standards to check the Style of each file, it should match correctly. When i look at the Travis build it looks good.

What should i exactly split or change?

Now this should pass the CodeStyle test that using https://github.com/joomla/coding-standards

avatar Bakual
Bakual - comment - 18 Mar 2014

Generally speaking: Codestyle PRs should be small so they can be reviewed fast and merged without further tests. Your PR deals with 39 files in a lot of plugins. That's not easy to review.

Either make one PR per plugin or one per codestyle issue. This way scope is limited and it's easy to review (and test if needed).

Important: Don't change any code at all in a Codestyle PR (like echoing instead of directly outputting).
Changing variable names is also bad (even if codestyle says to not use underscores in $_variable). It's possible that it may introduce BC issues.

When i look at the Travis build it looks good.

Sidenote: Travis doesn't check codestyle.

avatar shopfe
shopfe - comment - 18 Mar 2014

Okay, then i will split it into the plugins.

Important: Don't change any code at all in a Codestyle PR (like echoing instead of directly
outputting).
Changing variable names is also bad (even if codestyle says to not use underscores in
$_variable). It's possible that it may introduce BC issues.

The Renamings are full refactored, so it shouldn't accour a BC issue. ;)

Pls wait for my next PR's - this should be declined.

avatar Bakual
Bakual - comment - 18 Mar 2014

The Renamings are full refactored, so it shouldn't accour a BC issue. ;)

Famous last words :smile:
And then some crazy guy goes and extends the class and it fails.

I'm closing this PR and wait for the new ones. Thanks already in advance! It's good to see people caring about codestyle.

avatar Bakual Bakual - change - 18 Mar 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-03-18 12:54:01
avatar Bakual Bakual - close - 18 Mar 2014
avatar Bakual Bakual - close - 18 Mar 2014

Add a Comment

Login with GitHub to post a comment