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 readme #340

Merged
merged 4 commits into from
Oct 28, 2024
Merged

update readme #340

merged 4 commits into from
Oct 28, 2024

Conversation

olamidepeterojo
Copy link
Contributor

@olamidepeterojo olamidepeterojo commented Oct 20, 2024

What It Does
Updated typo in README
Consistent use of SDK name across SDKs

Fixes #326
Fixes #328
How to Test

Review Checklist
I certify that I have:

Additional Comments

@zFernand0 zFernand0 closed this Oct 21, 2024
@zFernand0 zFernand0 reopened this Oct 21, 2024
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.

Hey @olamidepeterojo,
Please make sure to signoff your commits 😋

To add your Signed-off-by line to every commit in this branch:

  1. Ensure you have a local copy of your branch by checking out the pull request locally via command line.
  2. In your local branch, run: git rebase HEAD~1 --signoff
  3. Force push your changes to overwrite the branch: git push --force-with-lease origin readme-update

Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.10%. Comparing base (385ee8c) to head (80a1520).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #340   +/-   ##
=======================================
  Coverage   89.10%   89.10%           
=======================================
  Files          65       65           
  Lines        3258     3258           
=======================================
  Hits         2903     2903           
  Misses        355      355           
Flag Coverage Δ
unittests 89.10% <ø> (ø)

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.

@olamidepeterojo
Copy link
Contributor Author

Thank you for the kind gesture and efforts in going through my PR... Will do accordingly

Signed-off-by: Olamide Ojo <[email protected]>
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! 😋

@zFernand0 zFernand0 requested a review from t1m0thyj October 21, 2024 16:58
@olamidepeterojo
Copy link
Contributor Author

Added the fix for this issue #328 to this PR

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.

Thanks for updating the readme as well.

I see that license headers have Zowe Python Client SDK

For a quick fix, I suggest doing a find-and-replace with your IDE.

Otherwise, you may need to enhance the existing scripts/license_header.py script to account for license header updates 😓

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 😋
just have one minor suggestion for the changelog entry for consistency 🙏

CHANGELOG.md Outdated
Comment on lines 144 to 145
- Fixed the inconsistent use of the SDK name across SDKs in all files by implementing the "find and replace" tool
- Therefore this fixes issue #328
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
- Fixed the inconsistent use of the SDK name across SDKs in all files by implementing the "find and replace" tool
- Therefore this fixes issue #328
- Fixed the inconsistent use of the SDK name across SDKs in all files by implementing the "find and replace" tool. [#328](https://github.com/zowe/zowe-client-python-sdk/issues/328)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, your suggestion is good, let me get it done right away

CHANGELOG.md Outdated
Comment on lines 141 to 144
- Fixed exception handling in session.py [#213](https://github.com/zowe/zowe-client-python-sdk/issues/213)

### Bug Fixes
Fixed the inconsistent use of the SDK name across SDKs in all files by implementing the "find and replace" tool. [#328](https://github.com/zowe/zowe-client-python-sdk/issues/328)
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
- Fixed exception handling in session.py [#213](https://github.com/zowe/zowe-client-python-sdk/issues/213)
### Bug Fixes
Fixed the inconsistent use of the SDK name across SDKs in all files by implementing the "find and replace" tool. [#328](https://github.com/zowe/zowe-client-python-sdk/issues/328)
- Fixed exception handling in session.py [#213](https://github.com/zowe/zowe-client-python-sdk/issues/213)
- Fixed the inconsistent use of the SDK name across SDKs in all files by implementing the "find and replace" tool. [#328](https://github.com/zowe/zowe-client-python-sdk/issues/328)

@olamidepeterojo olamidepeterojo force-pushed the readme-update branch 2 times, most recently from 9639b8e to 6082793 Compare October 28, 2024 15:14
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.

Because of all the force-push, I'm unable to determine when the changelog entry was added in 1.0.0-dev.11 instead of the Recent Changes. 😢

Would you mind moving it to the very top?

## Recent Changes

### Enhancements

- Turning of logger at the class-constructor level [#316] (https://github.com/zowe/zowe-client-python-sdk/issues/316)

### Bug Fixes

- Fixed the inconsistent use of the SDK name across SDKs in all files by implementing the "find and replace" tool. [#328](https://github.com/zowe/zowe-client-python-sdk/issues/328)

## `1.0.0-dev21`

Signed-off-by: Fernando Rijo Cedeno <[email protected]>
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! 😋

@olamidepeterojo
Copy link
Contributor Author

hey @zFernand0 , seems like you got it done already haha. would you still be needing i make the same changes?

@zFernand0
Copy link
Member

zFernand0 commented Oct 28, 2024

hey @zFernand0 , seems like you got it done already haha. would you still be needing i make the same changes?

Hey @olamidepeterojo,
I felt that I was being super annoying so that's why I tried to help out for once 😅
Apologies for all the requested changes...

And to answer you question, No.
I think there are no mo changes required/needed in order to get this PR merged in 🙏

Thanks for all the hard work 🙏

We just need one more review from the team before we get it merged 🥳
Apologies for the confusion... I thought we required more approvers

mering 😓

@zFernand0 zFernand0 merged commit c4e23ef into zowe:main Oct 28, 2024
33 checks passed
@olamidepeterojo
Copy link
Contributor Author

its no hassle @zFernand0 its nice working with you and its my pleasure.

could you kindly check my PR of issue #336. ive been waiting your review haha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Consistent Name for SDK Typo In README for Core Package Import
2 participants