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.

Sign up for GitHub

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

Jump to bottom

Drop php-cs-fixer and setup phpcs #423

Merged
merged 43 commits into from
Feb 5, 2024
Merged

Drop php-cs-fixer and setup phpcs #423

merged 43 commits into from
Feb 5, 2024

Conversation

connorhu
Copy link
Contributor

No description provided.

@connorhu connorhu force-pushed the feature/csfixer-3x branch 3 times, most recently from 545caf6 to d4d2c45 Compare January 26, 2024 16:22
derrabus
derrabus reviewed Jan 26, 2024
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@connorhu
Copy link
Contributor Author

Now it is OK. Is it necessary to reference php-cs-fixer in require-dev? There is workflow job, there is config, if someone needs php-cs-fixer then install it globally.

@greg0ire
Copy link
Collaborator

greg0ire commented Jan 27, 2024 •
edited
Loading

Would it make sense to switch to doctrine/coding-standard + https://github.com/doctrine/.github/blob/main/workflow-templates/coding-standards.yml instead of working on this?

Now it is OK. Is it necessary to reference php-cs-fixer in require-dev?

Is it causing any issue? Because it sure is more convenient for contributors to use what's provided by the project, especially if they have to ues a specific version to satisfy the build.

@connorhu
Copy link
Contributor Author

Let's see what it causes!

@connorhu connorhu force-pushed the feature/csfixer-3x branch 2 times, most recently from fb20c45 to 64349bf Compare January 27, 2024 15:17
greg0ire
greg0ire reviewed Jan 27, 2024
View reviewed changes
.github/workflows/coding-standards.yml Outdated Show resolved Hide resolved
.github/workflows/coding-standards.yml Outdated Show resolved Hide resolved
.github/workflows/coding-standards.yml Outdated Show resolved Hide resolved
@connorhu
Copy link
Contributor Author

Ok i got it how it works. Failing, no surprise.

@greg0ire
Copy link
Collaborator

Applying some of these rules will be a breaking change, and you should probably ignore them, like we did in doctrine/orm v2. To get their names, you can run phpcs with the -s flag

@connorhu
Copy link
Contributor Author

Most of the errors are phpdoc related errors (missing type hint). Nothing deadly serious. If you like it, I can fix the errors and we can make this PR a code sniffer migration.

@greg0ire
Copy link
Collaborator

greg0ire commented Jan 27, 2024 •
edited
Loading

Ah yes, fixing the errors with phpdoc is indeed a backwards-compatible solution, please do work on fixing the errors 👍

@connorhu connorhu force-pushed the feature/csfixer-3x branch 12 times, most recently from cb0da36 to 33f0195 Compare January 28, 2024 01:30
@connorhu
Code Sniffer: manual fix rule Squiz.NamingConventions.ValidVariableNa…
0466a63 
…me.NotCamelCaps
@connorhu
Code Sniffer: manual fix rule Squiz.Strings.DoubleQuoteUsage.ContainsVar
1bc0c03
@connorhu
Code Sniffer: manual fix rule Squiz.Commenting.FunctionComment.Missin…
12914a2 
…gParamName
@connorhu
Code Sniffer: manual fix rule SlevomatCodingStandard.TypeHints.Return…
a85ac5e 
…TypeHint.*
@connorhu
Code Sniffer: apply rule Squiz.Strings.DoubleQuoteUsage.NotRequired
06e39a6
@connorhu
Code Sniffer: manual fix visibility missing
ac2fad9
@connorhu
Code Sniffer: manual fix "Each index in a multi-line array must be on…
2145ae2 
… a new line"
@connorhu
Code Sniffer: manual fix "Space found before/after square bracket"
0630ea2
@connorhu
Code Sniffer: manual fix Squiz.Strings.DoubleQuoteUsage.ContainsVar
db1f5a0
@connorhu
Code Sniffer: manual fix PSR2.Methods.MethodDeclaration.Underscore
5371b08
@connorhu
Code Sniffer: manual fix SlevomatCodingStandard.TypeHints.ReturnTypeH…
c0ff1c1 
…int.MissingAnyTypeHint
@connorhu
Code Sniffer: manual fix Squiz.Strings.DoubleQuoteUsage.ContainsVar
4aa592b
@connorhu
Code Sniffer: apply rule Generic.Formatting.MultipleStatementAlignmen…
2ff2d11 
…t.NotSame
@connorhu
Code Sniffer: manual fix Squiz.Strings.DoubleQuoteUsage.ContainsVar
6b4fd1e
@connorhu
Code Sniffer: manual fix SlevomatCodingStandard.TypeHints.ReturnTypeH…
9116b6b 
…int.MissingTraversableTypeHintSpecification
@connorhu
Code Sniffer: apply rule 'Expected 1 line after "parent", found 0.'
87a013d
@connorhu
Code Sniffer: misc fixes
57b9c47
@connorhu
Code Sniffer: fix "Found multi-line doc comment with single line cont…
cc9cdaf 
…ent"
@connorhu
Code Sniffer: fix "Member variable ... is not in valid camel caps for…
bdebe1c 
…mat"
@connorhu
Revert "Code Sniffer: apply rule SlevomatCodingStandard.TypeHints.Dec…
cb46fd7 
…lareStrictTypes.DeclareStrictTypesMissing"

This reverts commit 0308030.
@connorhu
Revert "Code Sniffer: apply rule SlevomatCodingStandard.Commenting.Fo…
d073e31 
…rbiddenAnnotations.AnnotationForbidden"
@connorhu
add two exluded rules: forbidden annotation and strict types
ea294bf
@connorhu
convert phpdoc to one-liner
8cb11a5
@derrabus derrabus merged commit 2e08fb5 into beberlei:master Feb 5, 2024
8 checks passed
@connorhu
Copy link
Contributor Author

connorhu commented Feb 5, 2024

Thank you for merging it! Also, congratulations on the release of the ORM and DBAL!
I will continue with the ORM3 support PR cleanup.

@connorhu connorhu deleted the feature/csfixer-3x branch February 5, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

@greg0ire greg0ire greg0ire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@connorhu @greg0ire @derrabus