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

Return values instead of keys in discoverCommandsFromConfiguration. #5039

Merged
merged 4 commits into from Jan 24, 2022

Conversation

kporras07
Copy link
Contributor

discoverCommandsFromConfiguration function was returning keys from $commandList instead of values. This caused that the returned values were the path to the command file instead of the class and therefore the classes (and commands) are not loaded.

The fix included in this PR makes the commands to load again.

@weitzman
Copy link
Member

Odd that tests did not cover this. Looks like that line was introduced in #4696 so ping to @claudiu-cristea.

@greg-1-anderson
Copy link
Member

@kporras07 could you add a simple test to Drush using this configuration technique, to guard against future breakage?

@greg-1-anderson
Copy link
Member

FYIO, @kporras07 works for Pantheon, and I asked him to take over investigating this bug that I was looking at earlier this morning. #4696 broke various tests, but it wasn't immediately obvious because Drush 10.3 was in the lock file for that project, and I didn't have a "highest" test.

Copy link
Member

@greg-1-anderson greg-1-anderson left a comment

Choose a reason for hiding this comment

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

Tests look really good to me; thanks.

IDK if anyone else wants to see a separate PR with just the tests, for the failing case.

@weitzman weitzman merged commit 25b52ff into drush-ops:11.x Jan 24, 2022
@weitzman
Copy link
Member

LGTM. Thanks.

@kporras07 kporras07 deleted the fix-discover-commands-from-config branch January 24, 2022 22:53
ndf added a commit to ndf/drush that referenced this pull request Feb 2, 2022
* 11.x: (125 commits)
  Add a default value for the field widget choice in field:create (drush-ops#5060)
  Fix drush-ops#5058. Load/delete entities that are access controlled. (drush-ops#5059)
  Ignore services on invalid reference (drush-ops#5056)
  Fix tests on Drupal 10 (drush-ops#5054)
  Fix usage example in core:route command (drush-ops#5053)
  Back to dev.
  Prep for 11.0.4
  Update SecurityUpdateCommands endpoint (drush-ops#5043)
  Remove drush_get_global_options() (drush-ops#5046)
  Remove unused global options remote-host and remote-user. (drush-ops#5045)
  sql:sync minor cleanup (drush-ops#5044)
  Add back semver_example test on highest. (drush-ops#5031)
  Return values instead of keys in discoverCommandsFromConfiguration. (drush-ops#5039)
  Back to dev.
  Prep for 11.0.3
  Dont define entity-updates (drush-ops#5038)
  Back to dev.
  Prep for 11.0.2
  Remove dead code and re-enbale --partial test. (drush-ops#5036)
  Bump site-process for less verbose exceptions. (drush-ops#5034)
  ...

Merge conflicts
- src/Drupal/Commands/core/drush.services.yml
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.

None yet

3 participants