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

bpo-40563: Support pathlike objects on dbm/shelve #21849

Merged
merged 13 commits into from Sep 10, 2021
Merged

bpo-40563: Support pathlike objects on dbm/shelve #21849

merged 13 commits into from Sep 10, 2021

Conversation

audeoudh
Copy link
Contributor

@audeoudh audeoudh commented Aug 12, 2020

Refers to https://bugs.python.org/issue40563.

This work continues @hakancelik96's work from #20274.

https://bugs.python.org/issue40563

@the-knights-who-say-ni
Copy link

the-knights-who-say-ni commented Aug 12, 2020

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@audeoudh

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@ethanfurman
Copy link
Member

ethanfurman commented Sep 7, 2021

The CLA has been signed (according to the knight).

Lib/dbm/__init__.py Outdated Show resolved Hide resolved
Lib/test/test_dbm.py Outdated Show resolved Hide resolved
Lib/test/test_shelve.py Outdated Show resolved Hide resolved
Modules/_dbmmodule.c Outdated Show resolved Hide resolved
@audeoudh
Copy link
Contributor Author

audeoudh commented Sep 8, 2021

It was a long time I did not check this PR. I hope I did not forget something in the meantime.

I rebased the work on current main branch. I took @serhiy-storchaka's review into account.

I suppose I should squash before the merge, shouldn't I? Just let me know, I'll do it.

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Sep 8, 2021

I rebased the work on current main branch.

Force-pushing does not play well with GitHub and reviews. I would suggest to use git merge --no-ff main instead of git rebase main, as the review history is kept intact.

I suppose I should squash before the merge, shouldn't I? Just let me know, I'll do it.

No need; all PR's are squashed upon merging :)

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

The code LGTM. Please fix test_whichdb_ndbm and add tests for bytes paths.

Lib/dbm/__init__.py Outdated Show resolved Hide resolved
Lib/test/test_dbm.py Show resolved Hide resolved
Lib/test/test_dbm.py Outdated Show resolved Hide resolved
Lib/test/test_dbm.py Outdated Show resolved Hide resolved
Lib/test/test_dbm.py Outdated Show resolved Hide resolved
@audeoudh
Copy link
Contributor Author

audeoudh commented Sep 9, 2021

Force-pushing does not play well with GitHub and reviews. I would suggest to use git merge --no-ff main instead of git rebase main, as the review history is kept intact.

True for GitHub reviews. But merge --no-ff without rebase does not play well with git log itself.

But if all PR's are squased upon merging, it does not need to be rebased. Thanks for the tip.

Doc/library/dbm.rst Outdated Show resolved Hide resolved
Doc/library/dbm.rst Outdated Show resolved Hide resolved
Lib/test/test_dbm_dumb.py Show resolved Hide resolved
Lib/test/test_dbm_gnu.py Show resolved Hide resolved
Lib/test/test_dbm_ndbm.py Show resolved Hide resolved
Lib/test/test_shelve.py Show resolved Hide resolved
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

LGTM.

@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Sep 10, 2021
@serhiy-storchaka serhiy-storchaka merged commit 707137b into python:main Sep 10, 2021
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants