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

Commit 15c93909b51b5e4355bd60d8cdf37a428052f999 enforce declare(strict_types=1) - Was it intended ? #28

Closed
WengerK opened this issue Oct 1, 2024 · 5 comments

Comments

@WengerK
Copy link
Collaborator

WengerK commented Oct 1, 2024

Steps required to reproduce the problem

@FlorentTorregrosa in your commit 15c9390 you enable declare_strict_types therefore when using php-cs-fixer it will add declare(strict_types=1); at the top of each php files, wich is not a Drupal Best Practice.

Why having added this ? Was your intention to fix the declare(strict_types=1); format only ?

@FlorentTorregrosa
Copy link
Contributor

Hi @WengerK ,

As mentioned in #20, since https://www.drupal.org/project/coder/releases/8.3.23, declare(strict_types=1); is in the Drupal coding standards.

@WengerK
Copy link
Collaborator Author

WengerK commented Oct 1, 2024

Only to format

https://www.drupal.org/node/3407995

Introduce SlevomatCodingStandard.TypeHints.DeclareStrictTypes to check for this standard, only applying this sniff when a strict type is set, so we don't enforce the usage of strict typing.

@WengerK
Copy link
Collaborator Author

WengerK commented Oct 1, 2024

Enforcing strict declarative is pretty invasive - do you agree to only check format on it ? Like Drupal do ?

@FlorentTorregrosa
Copy link
Contributor

Oupsy, my bad.

As I already force it in my standards overrides I got warnings on all my projects, and as see it more and more in core and other contrib modules, I thought it was enforced.

So of course if Coder just enforce format if present, need to stick to that. And I will keep the enforce in my own overrides.

@WengerK
Copy link
Collaborator Author

WengerK commented Oct 1, 2024

I will revert a little of your commit then on #27 and merge it asap

@WengerK WengerK closed this as completed Oct 1, 2024
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

No branches or pull requests

2 participants