Return values instead of keys in discoverCommandsFromConfiguration.#5039
Merged
weitzman merged 4 commits intodrush-ops:11.xfrom Jan 24, 2022
Merged
Conversation
Member
|
Odd that tests did not cover this. Looks like that line was introduced in #4696 so ping to @claudiu-cristea. |
Member
|
@kporras07 could you add a simple test to Drush using this configuration technique, to guard against future breakage? |
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. |
greg-1-anderson
approved these changes
Jan 24, 2022
Member
greg-1-anderson
left a comment
greg-1-anderson
left a comment
There was a problem hiding this comment.
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.
Member
|
LGTM. Thanks. |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.