search

capi-workgroup / decisions

Discussion and voting on specific issues
4 stars 1 forks source link

Add Py_HashPointer #1

Closed encukou closed 6 months ago

encukou commented 6 months ago

The WG should decide if it likes Py_HashPointer as added here: https://github.com/python/cpython/pull/112096

vstinner commented 6 months ago

Since I wrote the PR and part of the C API Working Group, I will not vote on this decision.

Other members who are ok with the proposed API can check their checkbox below:

If you are not fine with it or if you skip the vote, please add a comment to explain why.

It's the first time that I create such checkbox list, I don't know how it works in terms of security. Or maybe each member should just leave a comment with a message like "+1", "LGTM" or something like that?

vstinner commented 6 months ago

It's the first time that I create such checkbox list, I don't know how it works in terms of security.

I can click on Guido's checkbox. I don't think that we should rely on checkboxes to count votes, but we can keep the checkboxes just to check if everybody voted.

zooba commented 6 months ago

The edit history tells you who changed the checkbox. I'm inclined to agree that posting a message with rationale is better anyway.

gvanrossum commented 6 months ago

I am okay with the API. (When I skimmed on the PR I had some nits, but that's a separate concern, and I actually don't care enough to engage on the PR.)

gvanrossum commented 6 months ago

I can click on Guido's checkbox. I don't think that we should rely on checkboxes to count votes, but we can keep the checkboxes just to check if everybody voted.

Presumably only C API WG members can change the checkboxes. I think we can trust each other.

iritkatriel commented 6 months ago

I don't have a problem with this API but I don't understand why we need both this and Py_HashDouble. The issue is for Py_HashDouble.

vstinner commented 6 months ago

I don't have a problem with this API but I don't understand why we need both this and Py_HashDouble. The issue is for Py_HashDouble.

See https://github.com/python/cpython/pull/112096#issuecomment-1842958858 for the rationale on adding Py_HashPointer().

vstinner commented 6 months ago

I suppose that this API was easy enough for a first vote, thanks for approving it :-) I merged my PR.