Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
serhiy-storchaka merged 13 commits into
python:mainpython/cpython:mainfrom
audeoudh:bpo-40563Copy head branch name to clipboard
Sep 10, 2021
Merged

bpo-40563: Support pathlike objects on dbm/shelve#21849
serhiy-storchaka merged 13 commits into
python:mainpython/cpython:mainfrom
audeoudh:bpo-40563Copy head branch name to clipboard

Conversation

@audeoudh

@audeoudh audeoudh commented Aug 12, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown

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
Copy Markdown
Member

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

Comment thread Lib/dbm/__init__.py Outdated
Comment thread Lib/test/test_dbm.py Outdated
Comment thread Lib/test/test_shelve.py Outdated
Comment thread Modules/_dbmmodule.c Outdated
@audeoudh

audeoudh commented Sep 8, 2021

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor

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 :)

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Comment thread Lib/dbm/__init__.py Outdated
Comment thread Lib/test/test_dbm.py
Comment thread Lib/test/test_dbm.py Outdated
Comment thread Lib/test/test_dbm.py Outdated
Comment thread Lib/test/test_dbm.py Outdated
@audeoudh

audeoudh commented Sep 9, 2021

Copy link
Copy Markdown
Contributor Author

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.

Comment thread Doc/library/dbm.rst Outdated
Comment thread Doc/library/dbm.rst Outdated
Comment thread Lib/test/test_dbm_dumb.py
Comment thread Lib/test/test_dbm_gnu.py
Comment thread Lib/test/test_dbm_ndbm.py
Comment thread Lib/test/test_shelve.py

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Sep 10, 2021
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.

7 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.