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

Extend coding standard with new rules #15

Merged
merged 7 commits into from
Feb 1, 2024

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Jan 30, 2024

Proposition to add these rules to our standard:

  • array_syntax: Force short syntax for array
  • list_syntax: Same for list
  • fully_qualified_strict_types: Remove namespace from classname when there is a use statement, and add missing backslash for global namespace
  • no_leading_import_slash: Remove leading slash from use statement
  • nullable_type_declaration_for_default_null_value: Add missing ? on type declaration for parameters defaulting to null. This will most likely be needed to avoid warnings in PHP 8.4.
  • yoda_style: forbid yoda style comparision. This replaces null === $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)

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>
@come-nc come-nc self-assigned this Jan 30, 2024
@come-nc come-nc marked this pull request as ready for review January 30, 2024 16:47
@come-nc come-nc added the enhancement New feature or request label Jan 30, 2024
provokateurin
provokateurin previously approved these changes Jan 30, 2024
Copy link
Member

@provokateurin provokateurin left a 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

nickvergessen
nickvergessen previously approved these changes Jan 30, 2024
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

I agree 👍

ChristophWurst
ChristophWurst previously approved these changes Jan 30, 2024
Altahrim
Altahrim previously approved these changes Jan 31, 2024
@nickvergessen
Copy link
Member

So that will be 1.2.0 right?

@come-nc
Copy link
Contributor Author

come-nc commented Feb 1, 2024

So that will be 1.2.0 right?

Yeah could be, is there a version number to bump somewhere?
Only CHANGELOG.md seems to have version numbers (and it’s outdated 😕 )

@nickvergessen
Copy link
Member

I would have expected composer.json but seems it's only the tag

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Copy link
Contributor Author

come-nc commented Feb 1, 2024

Added missing entries to the changelog file, good to merge.
(It seems this repos dismisses reviews on push, so you’ll need to approve again)

@nickvergessen nickvergessen merged commit 424cdd9 into master Feb 1, 2024
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the enh/extend-coding-standard branch February 1, 2024 12:54
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.

5 participants