-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix issue #156 to return proper message for active job #157
Conversation
Signed-off-by: Tian Na <tiantn@cn.ibm.com>
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.
Tested locally and looks good to me, thanks @tiantn!
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.
Approve. But trailing space at line 80 in src/api/JobUtils.ts
?
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 @tiantn! Tested locally and it works well 👍
$ zowe zftp ls sfbj TSU06008
No spool file available.
$ zowe zftp view asbj TSU06008
Command Error:
No spool files were available for job TSU06008. Try again after waiting a moment if the job is not yet in OUTPUT status.
src/api/JobUtils.ts
Outdated
fullSpoolFiles.push(spoolFileToDownload); | ||
if (jobDetails.spoolFiles) { | ||
for (const spoolFileToDownload of jobDetails.spoolFiles) { | ||
this.log.debug("Requesting spool files for job %s(%s) spool file ID %d", |
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.
Please remove trailing spaces to fix lint warning
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! 😋
I don't think I have anything to add that other reviewers haven't mentioned 😋
if (jobDetails.spoolFiles) { | ||
for (const spoolFileToDownload of jobDetails.spoolFiles) { |
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.
Wondering if we should push the condition down to the for-of
loop.
For example for (const spoolFileToDownload of jobDetails.spoolFiles ?? []) {
I'm not really requesting for this to be changed, it's just something that you may want to do in the future if you get lazy (like me) and don't want to bother with indentation 😋
Signed-off-by: Tian Na <tiantn@cn.ibm.com>
Quality Gate passedIssues Measures |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #157 +/- ##
==========================================
- Coverage 73.04% 72.80% -0.25%
==========================================
Files 80 80
Lines 983 989 +6
Branches 127 131 +4
==========================================
+ Hits 718 720 +2
+ Misses 265 248 -17
- Partials 0 21 +21 ☔ View full report in Codecov by Sentry. |
Thank you all for your good advice. I just removed the trailing spaces. 😊 |
Release succeeded for the The following packages have been published:
Powered by Octorelease 🚀 |
What It Does
How to Test
Review Checklist
I certify that I have:
Additional Comments