-
Notifications
You must be signed in to change notification settings - Fork 85
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
fixed start tso app and send tso app output formatting #2347
Conversation
Signed-off-by: jace-roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2347 +/- ##
=======================================
Coverage 91.16% 91.16%
=======================================
Files 636 636
Lines 18034 18039 +5
Branches 3879 3880 +1
=======================================
+ Hits 16440 16446 +6
+ Misses 1593 1592 -1
Partials 1 1 ☔ View full report in Codecov by Sentry. |
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.
changes LGTM! 😋
left a couple of comments, but nothing that should prevent this PR from being merged 🙏
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.
Thanks Jace for working on this fix! I left a suggestion regarding the current test cases, but the changes LGTM
Signed-off-by: jace-roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
…e properly passed to subsequent calls Signed-off-by: jace-roell <[email protected]>
Signed-off-by: Jace Roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
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.
Suggesting edits to the changelog entry.
Signed-off-by: jace-roell <[email protected]>
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.
Made some additional comments.
packages/cli/CHANGELOG.md
Outdated
@@ -2,6 +2,10 @@ | |||
|
|||
All notable changes to the Zowe CLI package will be documented in this file. | |||
|
|||
## Recent Changes | |||
|
|||
- BugFix: Improved output formatting for `zowe zos-tso start app` and `zowe zos-tso send app` by parsing and displaying relevant data rather than the entire JSON response [#2347](https://github.com/zowe/zowe-cli/pull/2347) |
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 we say that these are commands? Like: "Improved output formatting for the AAA and BBB commands by ... "
- Sentence should end w/ a period.
Signed-off-by: jace-roell <[email protected]>
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.
The new output LGTM 😋
but just left a question about a possible system test without the --rfj
flag
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.
sorry for being picky, but should we also have a system test for the non---rfj
approach?
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.
same as previous comment
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.
looks nice!
Signed-off-by: jace-roell <[email protected]>
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.
LGTM, thanks @jace-roell!
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.
Nice changelog!
Signed-off-by: Jace Roell <[email protected]>
Signed-off-by: jace-roell <[email protected]>
Quality Gate failedFailed conditions |
What It Does
Improved output formatting for
zowe zos-tso start app
andzowe zos-tso send app
Review Checklist
I certify that I have:
Additional Comments