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

implement 'ocdev component list' #225

Merged
merged 1 commit into from
Mar 19, 2018

Conversation

kadel
Copy link
Member

@kadel kadel commented Mar 9, 2018

fixes #53
fixes #191

implements:
ocdev component list/ocdev list - list component names in current application and their types

▶ ./ocdev list
You have deployed:
nodejs using the nodejs component
ffff using the nodejs component
wildfly using the wildfly component

TODO:

  • component.GetComponentType - needs to filter according to applicationName

@kadel kadel added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Mar 9, 2018
@kadel kadel force-pushed the component-list branch 2 times, most recently from 6a05518 to 76d5df8 Compare March 12, 2018 16:43
@kadel kadel removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Mar 12, 2018
Copy link
Contributor

@concaf concaf left a comment

Choose a reason for hiding this comment

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

This should return that there are no components when there are no components -

$ ocdev component list
You have deployed:

Copy link
Contributor

@concaf concaf left a comment

Choose a reason for hiding this comment

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

This message does not make sense -

$ ocdev component list
You have deployed:
nodejs using the nodejs component

Something like the following would be better:

$ ocdev component list
Application "nodejs" has the following components:
- nodejs
- nodejs2

@kadel
Copy link
Member Author

kadel commented Mar 13, 2018

This message does not make sense -

$ ocdev component list
You have deployed:
nodejs using the nodejs component

Why doesn't it make sense?
nodejs using the nodejs component
name of the component is the same as the type.

output can also look like this:

web-frontend using the nodejs component

see #191

@kadel
Copy link
Member Author

kadel commented Mar 13, 2018

This should return that there are no components when there are no components -

updated.
Now it shows

There are no components deployed.

@kadel
Copy link
Member Author

kadel commented Mar 13, 2018

Something like the following would be better:

$ ocdev component list
Application "nodejs" has the following components:
- nodejs
- nodejs2

this doesn't show what is the type of each component

@concaf
Copy link
Contributor

concaf commented Mar 13, 2018

Having the following message will be clearer IMO

$ ocdev component list
Application "nodejs" has the following components:
- nodejs, of type nodejs
- nodejs2, of type nodejs

@kadel
Copy link
Member Author

kadel commented Mar 13, 2018

Lets clear that in the original issue #191

@kadel
Copy link
Member Author

kadel commented Mar 14, 2018

can we sort out those messages outside this PR?
If this is the only issue I would merge this PR.

Right now it is implemented as it is described in the Issue. We can change those messages later.

@concaf concaf merged commit 1a2c976 into redhat-developer:master Mar 19, 2018
@kadel kadel deleted the component-list branch April 12, 2018 08:41
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.

ocd list should list components in current application implement ocdev component list to list all components
2 participants