Skip to content

bpo-27679: Remove set_bitfields()#14648

Merged
vstinner merged 1 commit into
python:masterfrom
shihai1991:bpo_27679
Jul 9, 2019
Merged

bpo-27679: Remove set_bitfields()#14648
vstinner merged 1 commit into
python:masterfrom
shihai1991:bpo_27679

Conversation

@shihai1991
Copy link
Copy Markdown
Member

@shihai1991 shihai1991 commented Jul 8, 2019 •
edited by bedevere-bot
Loading

@shihai1991
Copy link
Copy Markdown
Member Author

shihai1991 commented Jul 8, 2019

Looks like nobody need this set_bitfields() function.
If I was wrong , pls help me close this PR, thanks.

eamanu
eamanu approved these changes Jul 9, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

LGTM

@aeros
Copy link
Copy Markdown
Contributor

aeros commented Jul 9, 2019

Looks like nobody need this set_bitfields() function.
If I was wrong , pls help me close this PR, thanks.

In order to close a PR, simply enter in a final comment such as "Closing PR because ..." and instead of selecting the Comment option, choose the option to the left, Close and comment.

@shihai1991
Copy link
Copy Markdown
Member Author

shihai1991 commented Jul 9, 2019

Looks like nobody need this set_bitfields() function.
If I was wrong , pls help me close this PR, thanks.

In order to close a PR, simply enter in a final comment such as "Closing PR because ..." and instead of selecting the Comment option, choose the option to the left, Close and comment.

Thanks, got it;)

aeros
aeros reviewed Jul 9, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@aeros aeros 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.

In a search of the cpython repository for set_bitfields, this is the only occurrence. This is the initial message in the commit which first created the function 12 years ago: "Make _ctypes_test.c compile on Windows" without any further details or documentation.

If the integrations tests are still fully passing without it and it is not used in any other location, as far as I can tell, it can be safely removed. However, since this is not at all my area of expertise, I don't want to explicitly approve this PR, in case this function is still useful in some way that I'm unaware of. @vstinner would probably have a more definitive answer.

@vstinner vstinner merged commit 3a3db97 into python:master Jul 9, 2019
@vstinner
Copy link
Copy Markdown
Member

vstinner commented Jul 9, 2019

As soon as tests still pass, the change LGTM :-) Thanks.

lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@shihai1991 @lisroach
bpo-27679: Remove set_bitfields() from _ctypes_test (pythonGH-14648)
7c10e0b
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@shihai1991 @DinoV
bpo-27679: Remove set_bitfields() from _ctypes_test (pythonGH-14648)
d7ce797
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@aeros aeros aeros left review comments

@eamanu eamanu eamanu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@shihai1991 @aeros @vstinner @eamanu @the-knights-who-say-ni @bedevere-bot