-
Notifications
You must be signed in to change notification settings - Fork 87
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
Identical member names - PDS enhancement #2427
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[email protected]>
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Dismissed
Show dismissed
Hide dismissed
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Dismissed
Show dismissed
Hide dismissed
Signed-off-by: Pujal <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2427 +/- ##
=======================================
Coverage 91.37% 91.38%
=======================================
Files 639 639
Lines 18280 18298 +18
Branches 3847 3850 +3
=======================================
+ Hits 16703 16721 +18
Misses 1575 1575
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Pujal <[email protected]>
📅 Suggested merge-by date: 2/18/2025 |
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[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.
Left some comments, please review.
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Outdated
Show resolved
Hide resolved
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Outdated
Show resolved
Hide resolved
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Outdated
Show resolved
Hide resolved
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Outdated
Show resolved
Hide resolved
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Outdated
Show resolved
Hide resolved
packages/cli/__tests__/zosfiles/__unit__/copy/ds/Ds.handler.unit.test.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[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! 😋
I did leave some comments for possible improvements, but I do like the code as-is and works as expected 🙏
fromPds: string, | ||
toPds: string | ||
): Promise <boolean> { | ||
const sourceResponse = await List.allMembers(session, fromPds); |
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 do like how clean this implementation looks! 🙏
And I know this might be a bit picky, but in order to minimize the number of calls to the mainframe, could we cache this call?
await List.allMembers(session, fromPds);
or perhaps return it as part of the function too? 😅
That way, the copyPDS doesn't have to make the same call to get the list of members from the
fromPds` 😋
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[email protected]>
Signed-off-by: Pujal <[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.
Changes LGTM, thanks @pujal0909 for enhancing the copy PDS command!
What It Does
When copying PDSs, if the source and target have identical member names, the default behavior now is to prompt the user to confirm before overwriting the members contents (when the safe-replace option is not set to true). If the user uses the
replace
flag, it bypasses the prompt.How to Test
replace
option.Review Checklist
I certify that I have:
Additional Comments