close
Skip to content

gh-122854: Add Py_HashBuffer() function#122855

Merged
vstinner merged 4 commits intopython:mainfrom
vstinner:hashbytes
Aug 30, 2024
Merged

gh-122854: Add Py_HashBuffer() function#122855
vstinner merged 4 commits intopython:mainfrom
vstinner:hashbytes

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Aug 9, 2024 •
edited by github-actions Bot
Loading

@bedevere-app bedevere-app Bot mentioned this pull request Aug 9, 2024
Closed
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Aug 9, 2024

cc @pitrou @encukou

erlend-aasland
erlend-aasland reviewed Aug 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Did you intend to update Misc/stable_abi.toml also?

Comment thread Doc/c-api/hash.rst Outdated
Comment thread Doc/c-api/hash.rst Outdated
encukou
encukou reviewed Aug 12, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@encukou encukou left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you! This looks good, but, wasn't the name supposed to be Py_HashBuffer?

Comment thread Doc/c-api/hash.rst Outdated
picnixz
picnixz reviewed Aug 17, 2024
View reviewed changes
Comment thread Doc/c-api/hash.rst Outdated
Comment thread Objects/unicodeobject.c Outdated
Comment thread Lib/test/test_capi/test_hash.py
@vstinner vstinner changed the title gh-122854: Add Py_HashBytes() function gh-122854: Add Py_HashBuffer() function Aug 26, 2024
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Aug 26, 2024 •
edited
Loading

Thank you! This looks good, but, wasn't the name capi-workgroup/decisions#13 (comment)?

Oh, you're right! I modified the PR to use Py_HashBuffer(). I also rebased the PR to fix a merge conflict.

encukou
encukou reviewed Aug 27, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

With Erlend's suggestions, this should be good to merge!

Comment thread Modules/_testcapi/hash.c Outdated
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Aug 27, 2024

With Erlend's suggestions, this should be good to merge!

Fixed. Oh, I thought that I already addressed his review, but it was maybe on another similar PR.

I also replaced hash_bytes() with hash_buffer() in tests. Nicely spotted @encukou.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Aug 27, 2024

@erlend-aasland:

Did you intend to update Misc/stable_abi.toml also?

capi-workgroup/decisions#13 doesn't mention the stable ABI. Do you think that this API should be added to the stable ABI?

@vstinner vstinner enabled auto-merge (squash) August 30, 2024 15:19
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Aug 30, 2024

@erlend-aasland: I enabled auto-merge. If you consider that the function should be added to the limited C API 3.14, I can prepare a follow-up PR.

@vstinner vstinner merged commit d8e69b2 into python:main Aug 30, 2024
@vstinner vstinner deleted the hashbytes branch August 30, 2024 15:42
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Aug 30, 2024

Merged, thanks for reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments

@picnixz picnixz picnixz left review comments

@erlend-aasland erlend-aasland erlend-aasland left review comments

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

@abalkin abalkin Awaiting requested review from abalkin

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner

@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner

@tiran tiran Awaiting requested review from tiran

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner @encukou @picnixz @erlend-aasland