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

Fix and assert basic CS using CI #9246

Merged
merged 34 commits into from
Dec 16, 2023
Merged

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Dec 9, 2023

Assert basic coding style according to the latest standards and reduce merge conflicts in a long term.

This PR lands changes for non-functional (and mostly whitespace) changes only. I plan to fix more CS in upcoming PRs.

@mvorisek mvorisek force-pushed the fix_basic_cs branch 2 times, most recently from d7e4e47 to d47a3a8 Compare December 9, 2023 13:14
@alecpl
Copy link
Member

alecpl commented Dec 9, 2023

This is cool. I'd keep old switch/case indentation if possible. Not that I have strong opinion about the syntax, but I'd like to keep the diff less invasive. Please.

@mvorisek
Copy link
Contributor Author

mvorisek commented Dec 9, 2023

This is cool. I'd keep old switch/case indentation if possible. Not that I have strong opinion about the syntax, but I'd like to keep the diff less invasive. Please.

The case indentation is fixed by statement_indentation [1] PHP CS Fixer rule. It is not configureable for case, thus I cannot serve.

In a long them, this should pay off, as the PSR-12 CS [2] recommends/allows only this format. If you do a lot of merged into bugfix branches, I advise to add and run the PHP CS Fixer on these branches too, so the whitespace CS is the same and the diffs minimal.

[1] https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/blob/master/doc/rules/whitespace/statement_indentation.rst
[2] https://www.php-fig.org/psr/psr-12/#52-switch-case

@mvorisek mvorisek force-pushed the fix_basic_cs branch 2 times, most recently from cec26e4 to fffe315 Compare December 10, 2023 15:40
@mvorisek
Copy link
Contributor Author

@alecpl can this PR be merged?

@alecpl
Copy link
Member

alecpl commented Dec 12, 2023

Probably not before this weekend.

@mvorisek mvorisek marked this pull request as draft December 12, 2023 13:09
@alecpl alecpl marked this pull request as ready for review December 16, 2023 14:22
@mvorisek mvorisek marked this pull request as draft December 16, 2023 14:32
@alecpl alecpl marked this pull request as ready for review December 16, 2023 14:37
@alecpl alecpl merged commit a8707ae into roundcube:master Dec 16, 2023
14 checks passed
@mvorisek mvorisek deleted the fix_basic_cs branch December 16, 2023 14:40
@mvorisek mvorisek restored the fix_basic_cs branch December 16, 2023 15:06
@mvorisek mvorisek deleted the fix_basic_cs branch December 16, 2023 15:19
@mvorisek mvorisek restored the fix_basic_cs branch December 16, 2023 15:20
@mvorisek mvorisek deleted the fix_basic_cs branch December 16, 2023 15:23
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.

2 participants