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

Fixes #711: Adding ide:list-mine command. #712

Merged
merged 7 commits into from
Nov 3, 2021

Conversation

grasmash
Copy link
Contributor

@grasmash grasmash commented Nov 2, 2021

Motivation

Fixes #711

Proposed changes

  • Add a new ide:list-mine command.
  • Rename ide:list to ide:list-app, leave an ide:list for backwards compatibility.

Alternatives considered

Change behavior of ide:list to make application argument optional.

Testing steps

  1. Follow the contribution guide to set up your development environment.
  2. Clear the kernel cache to pick up new and changed commands: ./bin/acli ckc
  3. Run ide:list-app
  4. Run ide:list-mine

Merge requirements

  • Bug, enhancement, or breaking change label applied
  • Manual testing by a reviewer

@grasmash grasmash added dependencies Pull requests that update a dependency file enhancement labels Nov 2, 2021
@github-actions github-actions bot removed the dependencies Pull requests that update a dependency file label Nov 2, 2021
@grasmash grasmash marked this pull request as ready for review November 2, 2021 23:48
@grasmash
Copy link
Contributor Author

grasmash commented Nov 3, 2021

If we were going to model our command names off of Cloud API's patterns, we'd actually name the commands something like:

  • account:ides
  • application:ides
  • ide:create
  • ide:delete
  • ide:info
  • ide:open

Copy link
Contributor

@anavarre anavarre left a comment

Choose a reason for hiding this comment

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

./acli.phar ide:list-mine
 =======================================================================
  IDEs
 =======================================================================
  drupal7
  UUID: 4d287d5c-5e7d-4daa-b7c8-0c64c44618b1
  Application: MyD7App
  Subscription: MyD7App
  IDE URL: https://4d287d5c-5e7d-4daa-b7c8-0c64c44618b1.ide.ahdev.cloud
  Web URL: https://4d287d5c-5e7d-4daa-b7c8-0c64c44618b1.web.ahdev.cloud
 =======================================================================
  d8isnotdead
  UUID: c7c2134c-7192-4c50-800e-1c72b592edc7
  Application: Main App
  Subscription: Main App
  IDE URL: https://c7c2134c-7192-4c50-800e-1c72b592edc7.ide.ahdev.cloud
  Web URL: https://c7c2134c-7192-4c50-800e-1c72b592edc7.web.ahdev.cloud
 =======================================================================
  test
  UUID: abab4dd0-47d8-4db9-ad39-12ab33f35adf
  Application: Main App
  Subscription: Main App
  IDE URL: https://abab4dd0-47d8-4db9-ad39-12ab33f35adf.ide.ahdev.cloud
  Web URL: https://abab4dd0-47d8-4db9-ad39-12ab33f35adf.web.ahdev.cloud
 =======================================================================
 =======================================================================

grasmash and others added 2 commits November 3, 2021 13:18
Co-authored-by: Dane Powell <git@danepowell.com>
@grasmash grasmash merged commit a5db854 into acquia:master Nov 3, 2021
@grasmash grasmash deleted the issue-711-ide-list-mine branch November 3, 2021 17:31
Copy link
Contributor

@danepowell danepowell left a comment

Choose a reason for hiding this comment

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

I was already working on a review when you merged 😬 I'll still leave these comments, maybe we can follow up.

*/
protected function execute(InputInterface $input, OutputInterface $output) {
$acquia_cloud_client = $this->cloudApiClientService->getClient();
$account_ides = $acquia_cloud_client->request('get', '/account/ides');
Copy link
Contributor

Choose a reason for hiding this comment

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

Opened an upstream issue to add this endpoint: typhonius/acquia-php-sdk-v2#208

Not a blocker for this PR, but feel free to weigh in there.

$table->setStyle('borderless');
$table->setHeaders(['IDEs']);
foreach ($account_ides as $ide) {
$app_url_parts = explode('/', $ide->_links->application->href);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it should use the same output format as ide:info. Can we refactor this and ide:info to use a common method that accepts an array of IDE resources and pretty-prints a table?

Maybe even use it for ide:list as well, with a verbose parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed CLI-530 so your feedback is not lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add command to list all of "my" IDEs across all apps
3 participants