- Issue created by @mstrelan
- Merge request !9930Issue #3483057: Add void return type to all hook_install and hook_uninstall implementations β (Closed) created by mstrelan
- π³π±Netherlands bbrala Netherlands
Almost got scared at this one, went through all function, and the count did add up, nothing missed.
One thing that has been missed though is perhaps some tests:
Drupal/FunctionalTests/Install/
has multiple tests that actually write an install function to file. Shouldn't those also be changed to add the return?Example:
file_put_contents("$path/my_distribution.install", "<?php function my_distribution_install() {\Drupal::entityTypeManager()->getStorage('path_alias')->create(['path' => '/user/1', 'alias' => '/root-user'])->save();}");
- π¦πΊAustralia mstrelan
Thanks for the review. I'm not sure there's much point updating these files generated in tests. The point of this issue is to reduce the baseline and maybe one day get to a point where there is a phpcs rule that enforces this, but the contents of this generated file doesn't really conform to any other coding standards. It will never be scanned by phpcs nor phpstan. It also adds to the cognitive load for reviewer or committer as it looks different to the rest of the diff.
Setting back to Needs Review, if you feel strongly set it back again and I'll work on it.
- πΊπΈUnited States smustgrave
Would agree with #5. Since these are generated for tests believe should be fine. If we had any hooks generated from starterkit for live themes then I'd lean towards updating.
Current changes seem fine to me though.
- π³πΏNew Zealand quietone
I got the same results and had the same questions as @bbrala which is answered in #6. Answers that I agree with.
Assigning to myself to commit in a day or two.
-
quietone β
committed 42c86202 on 11.x
Issue #3483057 by mstrelan, bbrala, smustgrave: Add void return type to...
-
quietone β
committed 42c86202 on 11.x