Skip to content

Conversation

@susnux
Copy link

@susnux susnux commented Oct 2, 2025

@susnux susnux added the enhancement New feature or request label Oct 2, 2025
kesselb
kesselb previously approved these changes Oct 2, 2025
@susnux susnux requested a review from kesselb October 2, 2025 09:27
kesselb
kesselb previously approved these changes Oct 2, 2025
ChristophWurst
ChristophWurst previously approved these changes Oct 2, 2025
nickvergessen and others added 2 commits October 6, 2025 12:41
fix: Define risky by default and only use the rule then
@ChristophWurst ChristophWurst dismissed stale reviews from kesselb and themself via 179299c October 6, 2025 11:21
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure this is important enough to enable risky changes in our coding standard. Application should be able to rely on the cs-fixer not breaking anything.

@susnux susnux requested a review from ChristophWurst October 6, 2025 14:21
@ChristophWurst
Copy link
Member

I’m not sure this is important enough to enable risky changes in our coding standard. Application should be able to rely on the cs-fixer not breaking anything.

This is a good point I hadn't realized in #50. Generally enabling risky rules by default sounds like a bad idea.
Could you keep it off by default and still enable the logical operators when someone opts in?

@nickvergessen
Copy link
Member

Could you keep it off by default and still enable the logical operators when someone opts in?

We can, the sad thing is that if the config can not access the passed --allow-risky to only define the rule when the user opted-in.
So my vote would be to not include the rule for now and run it manually once in server to clear it. I've never seen it outside of server, as it's very uncommon anyway.

@nickvergessen
Copy link
Member

nickvergessen commented Oct 7, 2025

On that note: https://github.com/symfony/symfony/blob/7.4/.php-cs-fixer.dist.php#L50

I guess it's okay, as the only risky thing is if we add a new risky rule at some point. Because going forward it would prevent anyone from using and/or in first place, so there is no risk from that rule anymore.

@ChristophWurst
Copy link
Member

So my vote would be to not include the rule for now and run it manually once in server to clear it. I've never seen it outside of server, as it's very uncommon anyway.

This makes most sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal use logical_operators

6 participants