-
Notifications
You must be signed in to change notification settings - Fork 86
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
addEditOptions #2027
addEditOptions #2027
Conversation
Signed-off-by: Amber Torrise <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2027 +/- ##
=======================================
Coverage 91.00% 91.00%
=======================================
Files 635 635
Lines 18601 18601
Branches 3866 3866
=======================================
Hits 16928 16928
Misses 1672 1672
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Amber Torrise <[email protected]>
…options are apart of download not really edit so those will be tested in download right? Signed-off-by: Amber Torrise <[email protected]>
@t1m0thyj @zFernand0 do I have to add tests for these new options? wondering because the edit option is essentially performing zowe-cli/packages/cli/src/zosfiles/edit/Edit.utils.ts Lines 167 to 178 in f937576
don't the download tests cover this? ie: zowe-cli/packages/cli/__tests__/zosfiles/__unit__/download/ds/Dataset.handler.unit.test.ts Line 166 in f937576
|
Signed-off-by: Amber Torrise <[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.
Looks pretty good @ATorrise, left a few comments 🙂
...li/__tests__/zosfiles/__integration__/edit/ds/__snapshots__/edit.ds.integration.test.ts.snap
Outdated
Show resolved
Hide resolved
...ntegration__/cmd/__tests__/integration/cli/auth/Cmd.cli.auth.login.fruit.integration.test.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
…o addEditOptions
Signed-off-by: Amber Torrise <[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 @ATorrise!
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 for addressing my previous comments :) Found a minor issue when testing.
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[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! 😋
Tested locally and works perfectly fine!
I don't think we must have system tests specific to these options.
They seem self explanatory and we are properly forwarding them to z/OSMF 😋
I'm fine if you want to take a moment to write some though 😋
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 @ATorrise!
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Release succeeded for the The following packages have been published:
Powered by Octorelease 🚀 |
What It Does
Adds the ability to specify that the file should be downloaded as
--binary
or in a specific--encoding
with added options.How to Test
Review Checklist
I certify that I have:
Additional Comments