acquia / cli

Command-line interface for Acquia Cloud Platform products
https://docs.acquia.com/acquia-cli/
GNU General Public License v2.0
42 stars 47 forks source link

Per-command dependency injection #1679

Closed danepowell closed 6 months ago

danepowell commented 6 months ago

I'm frequently discouraged from adding new commands if they would require adding a new dependency to the service container, since this requires adding the dependency to our already-massive CommandBase constructor as well as all the places its invoked. This is a pain to maintain.

And of course, there are performance considerations in building a service container with every possible dependency for every possible command every time the application is run.

It would be a lot better to only inject the dependencies necessary for a specific command when that command is run. This PR sets a precedent for that pattern and makes some progress by removing the Guzzle Client and Logstream Manager as base dependencies.

I'd really love to kill the CommandBase entirely, but I haven't figured out how to handle dependencies that are universal or nearly-universal, such as the Telemetry and Credentials helpers.

codecov[bot] commented 6 months ago

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (6d963af) 91.30% compared to head (c1fce4c) 91.27%.

Files Patch % Lines
src/Command/CommandBase.php 91.81% 14 Missing :warning:
src/Command/Pull/PullCommand.php 33.33% 4 Missing :warning:
src/Command/Pull/PullDatabaseCommand.php 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1679 +/- ## ============================================ - Coverage 91.30% 91.27% -0.03% - Complexity 1786 1789 +3 ============================================ Files 122 122 Lines 6373 6375 +2 ============================================ Hits 5819 5819 - Misses 554 556 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.