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

Update doc strings #307

Closed
wants to merge 3 commits into from
Closed

Update doc strings #307

wants to merge 3 commits into from

Conversation

pem70
Copy link
Collaborator

@pem70 pem70 commented Jul 8, 2024

What It Does
Update doc strings for all functions as required by [#279]

How to Test

Review Checklist
I certify that I have:

Additional Comments

pem70 added 2 commits July 8, 2024 15:13
@pem70 pem70 requested review from t1m0thyj and zFernand0 July 8, 2024 19:16
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.90%. Comparing base (19297ff) to head (a2bcf84).
Report is 146 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #307   +/-   ##
=======================================
  Coverage   89.90%   89.90%           
=======================================
  Files          53       53           
  Lines        2713     2713           
=======================================
  Hits         2439     2439           
  Misses        274      274           
Flag Coverage Δ
unittests 89.90% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@JTonda JTonda requested a review from anaxceron July 10, 2024 15:07
@zFernand0 zFernand0 changed the base branch from main to Auth_Type July 10, 2024 16:38
@zFernand0
Copy link
Member

zFernand0 commented Jul 10, 2024

Hey @anaxceron,
Here is the live site with the changed that are in the main branch.
https://zowe-client-python-sdk.readthedocs.io/

We don't have the equivalent of Netlify for PR builds yet 😢
But hopefully it helps. 😋


UPDATE:

We do have builds for every PR 😋
https://zowe-client-python-sdk--307.org.readthedocs.build/en/307/

Thanks to @t1m0thyj for helping me find this! 🙏🏽

Base automatically changed from Auth_Type to main July 12, 2024 19:12
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 @pem70!

Left minor comments related to consistency and documenting the encoding option.

@@ -8,6 +8,8 @@ All notable changes to the Zowe Client Python SDK will be documented in this fil

- Included support for `AUTH_TYPE_CERT_PEM` and `AUTH_TYPE_NONE` in `session` [#291] (https://github.com/zowe/zowe-client-python-sdk/issues/291) and [#296] (https://github.com/zowe/zowe-client-python-sdk/issues/296)

- Updated all functions descriptions to be consitent [#279] (https://github.com/zowe/zowe-client-python-sdk/issues/279)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Updated all functions descriptions to be consitent [#279] (https://github.com/zowe/zowe-client-python-sdk/issues/279)
- Updated doc strings for all functions to be consistent [#279] (https://github.com/zowe/zowe-client-python-sdk/issues/279)

data: str
Content to be written
encoding:
Specifies encoding schema
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Specifies encoding schema
Specifies encoding name (e.g. IBM-1047)

output_file:str
Name of the local file to create
with_prefixes:boolean
If true, include a four big endian bytes record length prefix. The default is False
Copy link
Member

Choose a reason for hiding this comment

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

Should we use formatting consistent with get_binary_content?

Suggested change
If true, include a four big endian bytes record length prefix. The default is False
If true, include a four big endian bytes record length prefix. Default: False

dataset_name: str
Name of the dataset to be created
encoding: str
Specifies the encoding schema
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Specifies the encoding schema
Specifies the encoding name (e.g. IBM-1047)

Comment on lines +50 to +53
file_system_name: str
the name for the file system
options: dict
specifies file system attributes
Copy link
Member

Choose a reason for hiding this comment

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

I think we should use sentence case to make the descriptions consistent

Suggested change
file_system_name: str
the name for the file system
options: dict
specifies file system attributes
file_system_name: str
Name of the file system
options: dict
Specifies file system attributes

Comment on lines +97 to +102
file_path: str
file_path of the file to add
type: str
"file" or "dir"
mode: str
Ex:- rwxr-xr-x
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
file_path: str
file_path of the file to add
type: str
"file" or "dir"
mode: str
Ex:- rwxr-xr-x
file_path: str
Path of the file to add
type: str
Specify either "file" or "dir"
mode: str
Unix permission string (e.g. `rwxr-xr-x`)

data: str
Contents to be written
encoding: str
Specifies the encoding schema
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Specifies the encoding schema
Specifies the encoding name (e.g. IBM-1047)

Comment on lines +325 to +332
member_pattern: str
Filters members by name pattern.
member_start: str
The starting point for listing members.
limit: int
The maximum number of members returned.
attributes: str
The member attributes to retrieve.
Copy link
Member

Choose a reason for hiding this comment

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

Trailing periods should be removed for consistency

@@ -325,6 +381,7 @@ def copy_data_set_or_member(
Enqueue type for the dataset to copy from
replace: bool
If true, members in the target data set are replaced.
Copy link
Member

Choose a reason for hiding this comment

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

Trailing periods should be removed for consistency

dataset_name: str
The name of the dataset
stream: bool
Specifies
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Specifies
Specify True to return content as a stream object instead of a string. Default: False

@pem70
Copy link
Collaborator Author

pem70 commented Jul 15, 2024

Merged into a combined pull request.

@pem70 pem70 closed this Jul 15, 2024
@t1m0thyj t1m0thyj mentioned this pull request Jul 17, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants