-
Notifications
You must be signed in to change notification settings - Fork 176
Add services column to docker app ls command #757
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't understand the change to installerContext. Would at least request a comment if there's a legitimate reason.
@@ -22,55 +26,78 @@ import ( | |||
var ( | |||
listColumns = []struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't took time to create a formatter for app ls command, but we should for homogeneity. Will look into this in a future PR.
737346c
to
a6b8ddc
Compare
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
- List all the required services and the running services - Tries to call status json action on all installed app Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
… the last command still using it Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
a6b8ddc
to
f84667a
Compare
Codecov Report
@@ Coverage Diff @@
## master #757 +/- ##
=========================================
- Coverage 70.07% 69.58% -0.5%
=========================================
Files 64 64
Lines 3519 3567 +48
=========================================
+ Hits 2466 2482 +16
- Misses 727 757 +30
- Partials 326 328 +2
Continue to review full report at Codecov.
|
PTAL |
- What I did
The current
docker app ls
command relies only on parsing claims in the local claim store and its last action, so it is totally uncorrelated with the current app status.- How I did it
ls
command aggregates all those services, and counts how many are running and how many are desired (from the bundle).- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory)