search

lstrojny / functional-php

Primitives for functional programming in PHP
MIT License
1.98k stars 204 forks source link

PSR-2 code style #184

Closed alexeyshockov closed 5 years ago

alexeyshockov commented 5 years ago

PSR-2 has been adopted by most of PHP libraries in the past years, so I think it's a good idea to change the formatting according to this standard.

There are not a lot of changes, though, but I think it's important to apply them, because it allows an arbitrary developer to read code faster and not stumble on the things with different (from most of other library) formatting.

All the changes in the PR come from PHP-CS-Fixer tool with a default config (that follows PSR-2).

lstrojny commented 5 years ago

Good idea! Could you also add php-cs-fixer rules for it + travis integration (so fail the build if it finds changes)?

alexeyshockov commented 5 years ago

Update the PR, included a PHP_CodeSniffer config and Travis integration.

PHP-CS-Fixer is intended to fix code, linter is not their main goal. That's why for continuous integration I added phpcs as a linter.

It also found a few new issue with the code, some of them I fixed and some added as exceptions in phpcs.xml.dist.

alexeyshockov commented 5 years ago

Ah, just saw the discussion in #176. It was not obvious by the title that that PR is also related to the coding style topic.

As I said before, IMO for CI purposes PHP_CodeSniffer is better, but having a PHP-CS-Fixer config in the repo for such a tricky thing like 'native_function_invocation' is good and doesn't conflict with phpcs.

lstrojny commented 5 years ago

Mhhh, having two tools for a similar purpose will eventually conflict (we have that @InterNations at it can be a problem). So I would prefer to only use the php-cs-fixer, not code sniffer.

alexeyshockov commented 5 years ago

Agree that using both tools for the same task is not ideal. But not only have many things in common, but also some different checks and (most important) different goals.

I used both tools for the library's code, php-cs-fixer first to fix the code, and phpcs after that to integrate in with Travis CI. And you can see (54e7a15f93529b473a3ffb21dc6e9dcc55aa7b9d) that phpcs found additional issues, that php-cs-fixer simply doesn't warn about.

That's also why I said that it's better choice IMO to stick with phpcs for most of the checks, and also have php-cs-fixer for some specific checks (like 'native_function_invocation'). I think this is the best from both worlds. So main coding rules are checked by phpcs and only specific rules with php-cs-fixer.

Take a look at 'coding-style' script, it run both of them now. What do you think?

lstrojny commented 5 years ago

OK, convinced, let’s do it. Thank you!