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

Select members enhancement #2359

Merged
merged 30 commits into from
Nov 25, 2024
Merged

Select members enhancement #2359

merged 30 commits into from
Nov 25, 2024

Conversation

pujal0909
Copy link
Contributor

@pujal0909 pujal0909 commented Nov 11, 2024

What It Does
Created a new command zowe zos-files download all-members-matching that allows the user to input patterns to download specific members of a PDS (similar to zos-files download data-sets-matching).
#2175

How to Test
Run the command zowe zos-files download all-members-matching with specific data set and patterns to download specific members.
Ex. zowe zos-files download amm "test-pds" "mem*"

Review Checklist
I certify that I have:

Additional Comments

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 93.47826% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.27%. Comparing base (4a894f4) to head (d6f4767).
Report is 33 commits behind head on master.

Files with missing lines Patch % Lines
...osfiles/download/amm/AllMembersMatching.handler.ts 88.88% 2 Missing ⚠️
packages/zosfiles/src/methods/list/List.ts 95.45% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #2359    +/-   ##
========================================
  Coverage   91.27%   91.27%            
========================================
  Files         636      638     +2     
  Lines       18096    18141    +45     
  Branches     3796     3914   +118     
========================================
+ Hits        16517    16559    +42     
- Misses       1578     1581     +3     
  Partials        1        1            

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

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
@pujal0909 pujal0909 marked this pull request as ready for review November 14, 2024 13:33
@pujal0909 pujal0909 marked this pull request as draft November 14, 2024 13:36
@pujal0909 pujal0909 marked this pull request as draft November 14, 2024 13:36
@pujal0909 pujal0909 marked this pull request as draft November 14, 2024 13:36
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
@pujal0909 pujal0909 marked this pull request as ready for review November 14, 2024 14:34
@pujal0909 pujal0909 requested a review from gejohnston November 19, 2024 19:06
@adam-wolfe
Copy link
Contributor

adam-wolfe commented Nov 19, 2024

Functionally, this seems to work as expected. Is it possible to change the description for pattern in the help text? Right now it is:

   pattern               (string)

      The pattern or patterns to match data sets against. Also known as 'DSLEVEL'. The following special
      sequences can be used in the pattern:
      %: matches any single character
      *: matches any number of characters within a data set name qualifier (e.g. "ibmuser.j*.old" matches
      "ibmuser.jcl.old" but not "ibmuser.jcl.very.old")
      **: matches any number of characters within any number of data set name qualifiers (e.g. "ibmuser.**.old"
      matches both "ibmuser.jcl.old" and "ibmuser.jcl.very.old")
      However, the pattern cannot begin with any of these sequences. You can specify multiple patterns
      separated by commas, for example "ibmuser.**.cntl,ibmuser.**.jcl"

However, since we are filtering members, we don't really care about how these wildcards affect dsn qualifiers. If we can modify this description, I would change it to something like:

   pattern               (string)

      The pattern or patterns to match members against. The following special sequences can be used in the pattern:
      %: matches any single character
      *: matches any number of characters within a member name
      You can specify multiple patterns separated by commas, for example ""

I'm not sure what we would use for member names for the multiple patterns example, but the command does work with multiple patterns.

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
@pujal0909 pujal0909 force-pushed the select-members-enhancement branch from 41d649f to 2080f9f Compare November 20, 2024 15:03
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
@adam-wolfe
Copy link
Contributor

The changes to the help text look good. I would also remove Also known as 'DSLEVEL'. from the description for pattern, since we are filtering members, not data sets.

Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
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.

LGTM! 😋

left a couple of comments, but nothing that should impact the overall behavior 🙏

Great job! 🥳


> zdev files download amm "sys1.parmlib" "ady*,b*" --extension ".md"
14 members(s) were found matching pattern.ls

Member(s) downloaded successfully.
Destination: sys1/parmlib
image

packages/cli/CHANGELOG.md Outdated Show resolved Hide resolved
packages/zosfiles/CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Copy link
Member

@traeok traeok left a comment

Choose a reason for hiding this comment

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

Tested and functionality LGTM, thanks Pujal for the enhancement! I noticed a typo in the typedoc and had a suggestion for one of the strings in z/OS Files SDK.

packages/cli/src/zosfiles/-strings-/en.ts Outdated Show resolved Hide resolved
packages/zosfiles/src/constants/ZosFiles.messages.ts Outdated Show resolved Hide resolved
Signed-off-by: Pujal <pujal.gandhi@broadcom.com>
Copy link
Member

@traeok traeok 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 quickly addressing my feedback! Looks great 👍

Copy link

sonarcloud bot commented Nov 22, 2024

@awharn awharn requested a review from anaxceron November 22, 2024 20:47
@zFernand0 zFernand0 merged commit e684b78 into master Nov 25, 2024
19 checks passed
@zFernand0 zFernand0 deleted the select-members-enhancement branch November 25, 2024 14:10
@zFernand0 zFernand0 added release-current Indicates that there is no new functionality being delivered release-minor Indicates a minor feature has been added and removed release-current Indicates that there is no new functionality being delivered labels Nov 25, 2024
Copy link

Release succeeded for the master branch. 🎉

The following packages have been published:

  • npm: @zowe/zos-files-for-zowe-sdk@8.9.0
  • npm: @zowe/zos-workflows-for-zowe-sdk@8.9.0
  • npm: @zowe/zos-jobs-for-zowe-sdk@8.9.0
  • npm: @zowe/cli@8.9.0

Powered by Octorelease 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-minor Indicates a minor feature has been added released
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

6 participants