You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
First of all, I think this package is a very good idea. I especially like the fact that I can avoid doing my own, which was already in the planning phase. :)
As you can see from #1098, eventually I would like to automate the version-specific aspect as much as possible.
The problem I see is that when customizing some rule, I would have to keep in mind the PHP version as well. This might have been a "given" with version-specific rulesets, but it's particular annoying with the automated-version-ruleset.
Consider the following:
// in a PHP 8.3 project$companyRuleSet = Config\RuleSet\Php56::create()->withRules(Config\Rules::fromArray([
'void_return' => true,
]));
// in a PHP 5.6 project$companyRuleSet = Config\RuleSet\Php56::create()->withRules(Config\Rules::fromArray([
//'void_return' => true, <- we can't enable this for PHP 5.6
]));
// ^ In the two cases above, we'll need to have (and maintain) a different ruleset for each PHP version, rendering this package useless.// in any PHP project thanks to automatic version check$companyRuleSet = Config\RuleSet\PhpAuto::create()->withRules(Config\Rules::fromArray([
'void_return' => true, // <- this will still not work for e.g. PHP 5.6
]));
// ^ That skips one step, but there's still the PHP version problem
Solutions
A bit of a hack: generating some special value (e.g. object) which is than resolved correctly in the factory:
Implementing it as a child repo/package means skipping the usefulness of this repo. The 'since' thingy is otherwise rather readable.. could be something to consider in the current repo.
Applying the PHP-version-specific changes after defining the customization:
I don't think this would work though since some rules in this package seem to be deactivated based on preference rather than PHP-version (also following/considering Why forcing all the builtin rules to be configured? #2).
The cleanest approach I can think of (and that would still be backword-compatible):
// Backword-compatible change in PHP 7.0 ruleset. All other rulesets should be implemented similarlyclass Php70 {
publicstaticfunctioncreate() {
returnself::disableIncompatibleRules(Php71::create());
}
publicstaticfunctiondisableIncompatibleRules(RuleSet$set) {
return$set->withRules([
'void_return' => false,
// ...
]);
}
}
// Note: At the top of the hierarchy, `Php53::create()` would be calling `PhpBase::create()` where you define the defaults for all versions and an empty `disableIncompatibleRules`.// company php-cs-fixer config repoclass CompanyRuleSet {
// all the customizations we want
}
// company php-cs-fixer config file for any project$effectiveRuleSet = Config\RuleSet\PhpAuto::disableIncompatibleRules(CompanyRuleSet::create()); // TADA 🤩
Thoughts? Other ideas? If you like the 3rd approach, I'd gladly implement it in a PR.
The text was updated successfully, but these errors were encountered:
First of all, I think this package is a very good idea. I especially like the fact that I can avoid doing my own, which was already in the planning phase. :)
As you can see from #1098, eventually I would like to automate the version-specific aspect as much as possible.
The problem I see is that when customizing some rule, I would have to keep in mind the PHP version as well. This might have been a "given" with version-specific rulesets, but it's particular annoying with the automated-version-ruleset.
Consider the following:
Solutions
A bit of a hack: generating some special value (e.g. object) which is than resolved correctly in the factory:
Implementing it as a child repo/package means skipping the usefulness of this repo. The 'since' thingy is otherwise rather readable.. could be something to consider in the current repo.
Applying the PHP-version-specific changes after defining the customization:
I don't think this would work though since some rules in this package seem to be deactivated based on preference rather than PHP-version (also following/considering Why forcing all the builtin rules to be configured? #2).
The cleanest approach I can think of (and that would still be backword-compatible):
Thoughts? Other ideas? If you like the 3rd approach, I'd gladly implement it in a PR.
The text was updated successfully, but these errors were encountered: