Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PSR-2 code style #184

Merged
merged 4 commits into from Feb 28, 2019
Merged

PSR-2 code style #184

merged 4 commits into from Feb 28, 2019

Conversation

alexeyshockov
Copy link
Contributor

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
Copy link
Owner

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

@alexeyshockov
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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 (54e7a15) 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
Copy link
Owner

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

@lstrojny lstrojny merged commit 7a6dc26 into lstrojny:master Feb 28, 2019
@alexeyshockov alexeyshockov deleted the psr-2 branch February 28, 2019 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants