-
Notifications
You must be signed in to change notification settings - Fork 1
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
Extend coding standard with new rules #15
Conversation
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
…8.4) Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The added rules look reasonable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree 👍
So that will be 1.2.0 right? |
Yeah could be, is there a version number to bump somewhere? |
I would have expected composer.json but seems it's only the tag |
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
c178f60
Added missing entries to the changelog file, good to merge. |
Proposition to add these rules to our standard:
array_syntax
: Force short syntax for arraylist_syntax
: Same for listfully_qualified_strict_types
: Remove namespace from classname when there is ause
statement, and add missing backslash for global namespaceno_leading_import_slash
: Remove leading slash fromuse
statementnullable_type_declaration_for_default_null_value
: Add missing?
on type declaration for parameters defaulting tonull
. This will most likely be needed to avoid warnings in PHP 8.4.yoda_style
: forbid yoda style comparision. This replacesnull === $a
by$a === null
.I undertand that adding rules will mean a huge changelog at some point, and confuse a bit git blame and backports, but I still think it is worth it.
It avoids doing such changes by hand when cleaning up old code, it avoids arguing about these things in PR review. It improves consistency across the codebase and makes it easier to read.
And for
nullable_type_declaration_for_default_null_value
it avoids future deprecation warnings (https://wiki.php.net/rfc/deprecate-implicitly-nullable-types - Not voted yet)