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

Doc string enforcer and annotations #312

Merged
merged 48 commits into from
Jul 23, 2024
Merged

Doc string enforcer and annotations #312

merged 48 commits into from
Jul 23, 2024

Conversation

pem70
Copy link
Collaborator

@pem70 pem70 commented Jul 15, 2024

What It Does
Update current doc strings, introduce doc string check, and update annotation for all methods.
[#279] [#309] [#280]

How to Test

Review Checklist
I certify that I have:

Additional Comments

Copy link

codecov bot commented Jul 15, 2024

Codecov Report

Attention: Patch coverage is 99.16667% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.91%. Comparing base (2dbb721) to head (711ecd8).
Report is 97 commits behind head on main.

Files with missing lines Patch % Lines
.../zos_files/zowe/zos_files_for_zowe_sdk/datasets.py 95.83% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #312      +/-   ##
==========================================
- Coverage   90.11%   89.91%   -0.21%     
==========================================
  Files          53       53              
  Lines        2773     2816      +43     
==========================================
+ Hits         2499     2532      +33     
- Misses        274      284      +10     
Flag Coverage Δ
unittests 89.91% <99.16%> (-0.21%) ⬇️

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.

@pem70 pem70 requested review from t1m0thyj, traeok and zFernand0 July 15, 2024 20:52
@pem70 pem70 marked this pull request as ready for review July 15, 2024 20:53
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.

LGTM, thanks @pem70

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 that are unresolved from #307

CHANGELOG.md Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/file_system.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py Outdated Show resolved Hide resolved
src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py Outdated Show resolved Hide resolved
pem70 and others added 14 commits July 18, 2024 09:43
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Peizhao Mei <[email protected]>
Signed-off-by: pem70 <[email protected]>
@zFernand0
Copy link
Member

zFernand0 commented Jul 19, 2024

Hey @pem70,

When resolving these merge conflicts, remember to remove the basicConfig setup (removed in #313)

To avoid unintentional regressions in the future you can do either of the flowing:

  • Create new branches based off of the main branch.
  • If branches are created from branches other than main, select the corresponding base branch when creating PRs (and not the main branch). That way you don't have to worry about making the same changes in different branches.
    For example:
    • branch_A_based_on_main > branch_B_based_on_branch_A > branch_C_based_on_branch_B,
      image
      • PR#1: base: "main" <- compare: "branch_A_based_on_main"
      • PR#2: base: "branch_A_based_on_main" <- compare: "branch_B_based_on_branch_A"
      • PR#3: base: "branch_B_based_on_branch_A" <- compare: "branch_C_based_on_branch_B"

Doing the above should:

  1. Avoid possible regressions
  2. Allow PRs to be merged in any order
  3. Decrease the number of files changes on each PR
  4. Make PR reviews go faster

@zFernand0 zFernand0 requested a review from t1m0thyj July 22, 2024 14:16
Signed-off-by: pem70 <[email protected]>
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 @pem70!

@t1m0thyj t1m0thyj merged commit 297655c into main Jul 23, 2024
21 checks passed
@t1m0thyj t1m0thyj deleted the doc-string-enforcer branch July 23, 2024 18:29
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.

4 participants