• We should also add CI check.
• We probably also want @PHP54Migration and @PHP56Migration:risky
• And maybe even @Symfony or @Symfony:risky to automate as many style decisions as possible. Though then we will probably want to deactivate obnoxious rules like yoda_style.

• We should also add CI check.

Good point 👍

• We probably also want @PHP54Migration and @PHP56Migration:risky

SimplePie does not support PHP 5.4 anymore. And the only rule in @PHP56Migration:risky is pow_to_exponentiation, but we don't use pow() afaik.

• And maybe even @Symfony or @Symfony:risky to automate as many style decisions as possible. Though then we will probably want to deactivate obnoxious rules like yoda_style.

I don't have any particular preferences about this. It's enough for me if rules are defined and these can be implemented automatically by running a script.

SimplePie does not support PHP 5.4 anymore

Right, but we did not ever migrate to the newly introduced syntax so we still use long array syntax. PHP54MigrationSet would fix that.

Right, but we did not ever migrate to the newly introduced syntax so we still use long array syntax. PHP54MigrationSet would fix that.

Oh, wow, you are right. I would have guessed that the short array syntax was already defined in PSR-2. 😅

PSR does not cover many things we take for granted, which is why I suggested the Symfony ruleset (even though it contains some questionable or even choices potentially hindering comprehension of code like the yoda style – we read left to right so having the object that introduces the most information on the left side is clearly preferable. And in most cases that will be the variable. Really, Yoda style only makes sense if you are comparing multiple values against the same variable – but then maybe you should use in_array – or if you have Jedi powers and the order things are written in does not matter to you because you feel them at the same time).

• We should also add CI check.

Do you have a tip or example for me on how to make this happen?

• We probably also want @PHP54Migration and @PHP56Migration:risky

I've added all non-risky Migration rules, namely @PHP54Migration. I would not automate risky changes, but require a human to watch over changes.

• And maybe even @Symfony or @Symfony:risky to automate as many style decisions as possible. Though then we will probably want to deactivate obnoxious rules like yoda_style.

I would like to move these decisions to a separate PR. In this PR, I would like to have a basic set of rules that everyone can agree on.

Do you have a tip or example for me on how to make this happen?

I would probably add php-cs-fixer:3.8 (pinned version so that CI does not break when new version is released) along with cs2pr as described here as a new job in https://github.com/simplepie/simplepie/blob/master/.github/workflows/ci.yml.

Having it in composer dev dependencies would be nicer but would force us to stay with old version of php-cs-fixer, as the latest version requires PHP 7.4. Edit: Actually, it looks like having a separate composer file is recommended upstream: https://cs.symfony.com/#installation

I would like to move these decisions to a separate PR. In this PR, I would like to have a basic set of rules that everyone can agree on.

On the other hand, we are already changing the coding style significantly (replacing tabs with spaces) so this might be a best time for establishing a coding style once and for all. Each time we change it adds another obstacle to git blame (which has already been pretty battered by #722). Once this PR is merged we should add the commit hash to .git-blame-ignore-revs and hope it works.

nice work I prefer this coding style too

I've resolved the merge conflicts.

Actually, it looks like having a separate composer file is recommended upstream: https://cs.symfony.com/#installation

After reading up a bit, I have to say that I don't feel confident enough in this area. I would like to leave this topic to someone else.

And maybe even @Symfony or @Symfony:risky to automate as many style decisions as possible. Though then we will probably want to deactivate obnoxious rules like yoda_style.

@mblaney Any preferences about this? I could easily define some more rules in this PR.

sorry looks like I missed that this was ready to merge? happy with the composer change but not familiar with the symfony requirements and doesn't feel like something we need to add here?

happy with the composer change but not familiar with the symfony requirements and doesn't feel like something we need to add here?

We could remove the "friendsofphp/php-cs-fixer" library from "require-dev", but it allows every contributor to simply run bin/php-cs-fixer fix to fix all CS issues. Without this dependency the .php-cs-fixer.dist.php also makes no sense, where the CS rules are defined. I would recommend to keep this dependency.

sorry looks like I missed that this was ready to merge?

Yes, it is. As long as there are no change requests, this PR is ready for merge from my side.