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

Add strict typehinting to edge #55

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

danepowell
Copy link
Collaborator

I consider strict typehinting to be a best practice. We should adopt this in all of our projects. I'm adding this to edge since it's a heavy lift to adopt for existing projects without typehinting.

It might be such a large effort that we want to put it in a standalone "StrictTypes" standard or something.

I think there's also a Slevomat sniff to enforce the use of declare(strict_types=1) in every file, if we want to take it that far.

@TravisCarden
Copy link
Contributor

I consider strict typehinting to be a best practice. We should adopt this in all of our projects.

I'm on board!

I think there's also a Slevomat sniff to enforce the use of declare(strict_types=1) in every file, if we want to take it that far.

For posterity, that would be SlevomatCodingStandard.TypeHints.DeclareStrictTypes.

It might be such a large effort that we want to put it in a standalone "StrictTypes" standard or something.

Yeah. In fact, as you point out, even parameter/property/return type hints could be a big lift for some projects. What if we do this?

  1. Merge this PR as-is.
  2. Cut a final release on the 1.x branch, e.g., v1.1.0.
  3. Create a new major version branch (2.x) and merge AcquiaEdge into AcquiaDrupalStrict.
  4. Add SlevomatCodingStandard.TypeHints.DeclareStrictTypes to the now-empty AcquiaEdge.
  5. Get feedback from users on how difficult the new rule it would be to adopt and decide accordingly when and whether to merge it into AcquiaDrupalStrict.

@danepowell
Copy link
Collaborator Author

Let's do it! I'll merge this and see if I remember how to cut a release, I'll let you handle the comms, or maybe it needs to go through the ORCA team since ORCA will need to bump the coding standards to v2 at some point.

@danepowell danepowell merged commit 5fe637b into acquia:develop Mar 3, 2023
@danepowell
Copy link
Collaborator Author

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