Follow PSR2 rules

[keradus]

Bring to you by https://github.com/FriendsOfPHP/PHP-CS-Fixer

[bighappyface]

There they are: #132 (comment)

[keradus]

don't get it... it's ok for you to add that missing braces or not ?

[bighappyface]

@keradus would you add PSR-2 analysis to the build for enforcement? I think this is a great move, but it's going to force all other PRs to rebase and some to update for code style.

[keradus]

OK, please wait a sec

[keradus]

it will change deps, could you first take care of #139 ?

[keradus]

PHP CS Fixer does not support 5.3.3.
Do you really want to support that version ?

[bighappyface]

How about just using phpcs to inspect the code and fail the build?

I am fine with discontinuing support for 5.3.x, but it's a big move that would force us to 2.0.0, a move you also recommended with the PHP7 support updates in 1.4.0.

Before we go to 2.0.0, I think it's fair to try to fix some of the outstanding bugs with draft04 in this 1.x.x series so users on PHP 5.3.x can benefit.

[keradus]

Chaning minimal deps or supported envs is not BC break change. Semver is about interface, and changing env won't change the interface.

How about just using phpcs to inspect the code and fail the build?

One can do this that way. You could go that way if you want.
But then don't ask PHP CS Fixer author to do so.

One way or another I made the Travis green. Do as you wish.

[bighappyface]

@keradus I realize you are invested in PHP CS Fixer, and that the implementation you have for Travis basically accomplishes the desired ends.

One recommendation: put the code style analysis step before phpunit so that Travis can fail the build faster. Why test code that doesn't conform to the project code style?

Please consider that by employing phpcs only we gain the following:

1. PHP 5.1.2 minimum requirement versus the 5.3.6 requirement for PHP CS Fixer
2. Composer-based installation along side phpunit and phpdoc
   1. Developers can run vendor/bin/phpunit to run tests, why not vendor/bin/phpcs for code style analysis? The PHP CS Fixer approach only adds code style analysis to Travis.
3. More consistent invocation in Travis
   1. No inline check for PHP 5.3.6 (although the HHVM check may be necessary)
4. No new .php_cs file (e.g. vendor/bin/phpcs --standard=PSR2 instead)

If we did make PHP >= 5.4.x a requirement, I consider that to be an interface change when you don't take the phrasing too literally. If a developer can no longer use this package on their system because they are using PHP 5.3.x, that is an "interface" change in that it changes how the developer interacts with the software. It could mean whole operating system changes to get PHP 5.4.x or greater running on a server, and that's clearly a backwards compatibility change when you consider the numerous people using this package across various platforms. We can split hairs on this opinion all day, but it's not necessary. That is my opinion/interpretation of semver, and I respect yours.

I did not ask for you to introduce your personal code style fixer package. That was your call and I realize you had to spend time getting this PR to this point, but that doesn't mean we should just merge it and go. We have to consider the consumers of this package and all of the other contributors, and I know you appreciate this sentiment through your help reviewing other PRs. Remember, we are only checking the code style, not fixing it. I believe it is fair to obligate the contributor to follow the project code style. It is good for the contributor to be aware of code style and the nature of contribution, so fixing the code for them automatically doesn't benefit anyone. In this case, we aren't fixing the code for them anyway, so involving a full-featured code fixer seems to be beyond the scope of our needs.

Adding @justinrainbow @hakre @loucho for thoughts on this.

[keradus]

PHP 5.1.2 minimum requirement versus the 5.3.6 requirement for PHP CS Fixer

Your project does not support 5.1.2. It supports 5.3.0.
Supporting such an old and not maintable version is sad.

Composer-based installation along side phpunit and phpdoc (...) The PHP CS Fixer approach only adds code style analysis to Travis.

That is only the result of supporting 6 years old version of PHP
PHP CS Fixer can be installed via composer (in project or globally on user env) and used locally to fix code as well.
I was forced to remove it from your composer.json due to extra old PHP version that you support in this project.

More consistent invocation in Travis

Same as before.

No new .php_cs file

If one really want it could be removed and use cli args instead (like php-cs-fixer fix . --level=psr2).
But it is cool to not type it over and over again. Config file gives more options if one decided to use it.

That is my opinion/interpretation of semver, and I respect yours.

One day it was my interpretation too. Then it was changed by both ZF2 and Symfony. If world treat it differently then indeed, no need to focus on that.

I did not ask for you to introduce your personal code style fixer package.

#140 (comment)
Oh, btw, it is not my "personal" tool. And it is used by both Symfony and ZF.

Remember, we are only checking the code style, not fixing it. (...) fixing the code for them automatically doesn't benefit anyone

And that how the .php_cs file would be helpfull. If Travis detect invalid CS then one could easily fix it by just running php-cs-fixer fix

---

Both tools are great. If you ever decided to use one of them in your CI it would be cool move.

[bighappyface]

@keradus yikes.

For the record, I volunteered to help maintain this project after a lull in PR reviews. This is far more your project than mine as I only have merge commits whereas you have actually contributed code.

I am sorry to see you close this PR. You should try harder to work with people that are trying to work with you.

[keradus]

Just see no point about wasting time debating about nothing special.

I have offered little help with CS fixes, you point out your concerns about old PHP version, which is cool.

I see no need to force that PR to be merged or decided which CS tool this project gonna use, if anyone. You gave your arguments, I my agree with them or not, but I do respect your opinion.
This is only CS, no bugfix, no new feature. No need for such a big discussion that you want.