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

Fnf error reporting #2078

Merged
merged 18 commits into from
Mar 13, 2024
Merged

Fnf error reporting #2078

merged 18 commits into from
Mar 13, 2024

Conversation

ATorrise
Copy link
Contributor

@ATorrise ATorrise commented Mar 5, 2024

What It Does
Makes zosjob command errors more legible

How to Test

  • In vscode debugger:
    {
        "type": "node",
        "request": "launch",
        "name": "FNF messaging",
        "program": "${workspaceFolder}/zowe-cli/packages/cli/lib/main.js",
        "console": "integratedTerminal",
        // "args": ["files", "upload", "dtp", "src", "ibmuser.src"],
        "args": ["jobs", "submit", "lf", "noexist"],
        // "args": ["files", "upload", "ftds", "local", "remote"],
        "outputCapture": "std",
        "resolveSourceMapLocations": null
    },
  • compare output via command line:
    zowe jobs submit lf "noexist"
    zowe files upload ftds local "remote"

Review Checklist
I certify that I have:

Additional Comments

Copy link

codecov bot commented Mar 5, 2024

Codecov Report

Attention: Patch coverage is 77.02703% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 90.93%. Comparing base (10c5389) to head (152262f).
Report is 2 commits behind head on next.

Files Patch % Lines
...es/cli/src/zosjobs/submit/Submit.shared.handler.ts 77.02% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2078      +/-   ##
==========================================
- Coverage   90.93%   90.93%   -0.01%     
==========================================
  Files         616      616              
  Lines       17370    17377       +7     
  Branches     3674     3676       +2     
==========================================
+ Hits        15796    15801       +5     
- Misses       1573     1575       +2     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Amber Torrise <[email protected]>
@t1m0thyj t1m0thyj mentioned this pull request Mar 6, 2024
4 tasks
ATorrise added 3 commits March 6, 2024 12:26
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
@ATorrise ATorrise marked this pull request as ready for review March 6, 2024 18:04
@ATorrise ATorrise linked an issue Mar 7, 2024 that may be closed by this pull request
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

Looks pretty good, thanks @ATorrise! Left a few comments around how we could make the error details look more similar to output of files upload command.

packages/cli/src/zosjobs/submit/Submit.shared.handler.ts Outdated Show resolved Hide resolved
packages/cli/src/zosjobs/submit/Submit.shared.handler.ts Outdated Show resolved Hide resolved
Copy link
Member

@gejohnston gejohnston left a comment

Choose a reason for hiding this comment

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

Thanks for reworking this and reversing the behavior that was originally requested in the issue.

After Timothy's comments are addressed, I would approve the PR.

Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Copy link

Signed-off-by: Amber Torrise <[email protected]>
@ATorrise ATorrise requested a review from t1m0thyj March 13, 2024 14:50
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @ATorrise!

@JTonda JTonda requested a review from gejohnston March 13, 2024 15:04
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

changes do make sense!
LGTM! 😋

Comment on lines 17773 to +17778
"engines": {
"node": ">=18.12.0"
},
"engines": {
"node": ">=18.12.0"
},
Copy link
Member

Choose a reason for hiding this comment

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

a bit confused as to why this was added twice here 😋

@zFernand0 zFernand0 merged commit 62ac26d into next Mar 13, 2024
45 of 47 checks passed
@zFernand0 zFernand0 deleted the fnf-error-reporting branch March 13, 2024 16:24
Copy link

Release succeeded for the next branch. 🎉

The following packages have been published:

Powered by Octorelease 🚀

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

Successfully merging this pull request may close these issues.

File Not Found Messaging Inconsistent
4 participants