Problem/Motivation
Drush 11 is EOL. Drupal 10 requires Drush 12.5+.
Drush 12 works on Drupal 10 but not Drupal 11.
Drush 13 is required for Drupal 11 but will also work on Drupal 10.2+.
https://www.drush.org/13.x/install/#drupal-compatibility
Proposed resolution
Upgrade search_api Drush commands for Drush 12+.
The big things here are to use PHP Attributes instead of Annotations for Commands, and to use autodiscovery instead of drush.services.yml, and to use autowiring instead of service injection.
These things are supported by Drush 12 and required in Drush 13.
Remaining tasks
Basic steps that apply to this module are:
- Change namespace and class naming according to https://www.drush.org/12.x/commands/#auto-discovered-commands-psr4
- Add use statement for Drush PHP attribute classes.
- Use PHP attributes for Drush command definitions, as per https://www.drush.org/12.x/commands/#attributes-or-annotations
- Do not remove the Drush annotations yet - that will make these changes easier to review and easier to fix if a problem is noticed.
- Remove drush.services.yml and remove drush.services.yml from composer.json. Drush commands will be autoloaded.
- Use Autowire instead of dependency injection.
- Add parameter and return type hints to all methods. Turn on strict_types.
Issue fork search_api-3470199
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
tr commentedThere are no actual tests for the Drush commands, so the only things the tests will show are issues brought up by cspell, eslint, phpcs, and phpstan. Indeed, phpcs shows there was 1 missing space character. Fixed that.
You can see from the changes that none of the actual command methods were modified, so they work exactly as they did before. The only thing in the patch that had a potential to cause problems was adding the parameter and return type hints, and declaring strict_types. But there were no errors in the above tests. However, each command should be manually tested to ensure there are no runtime typing problems.
Comment #4
tr commentedComment #5
tr commentedComment #7
drunken monkeyThanks a lot for creating this issue, I had completely missed those changes in Drush. Also thanks a lot for already providing an MR with the necessary changes.
I manually ran all the commands, and also viewed their help, and made two fixes: one arguably unrelated one to the inner workings of the
sapi-icommand, the other just adding descriptions for all the arguments.Please test/review, if no-one objects I think I’ll just merge this in a week or so.
Comment #8
tr commentedIt doesn't seem right to RTBC my own patch, but I've looked at the changes you made and verified I missed the Arguments attributes and verified that you've added them correctly. So I think as far as I'm concerned this is ready to commit.
Comment #9
drunken monkeyGood to hear, thanks for reporting back!
Merged.
Thanks again!