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

refactor: scrub Python2 compatibility fossils #206

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

tseaver
Copy link
Member

@tseaver tseaver commented Jun 2, 2024

  • Disentangle checks for needed long_long/ulong_long helpers. Silences compiler warning for unused functions.
  • Replace Py2 compatibility macros w/ Python APIs.
  • Remove the local _compat.h header where they were defined, replacing includes of it everwhere with #include "Python.h".
    Note that the unrelated include/persistent/persistent/'compat.h header in the directory is still in place. No C header or module imports it directly, and none uses any macros defined in it indirectly.

This work is in preparation for adding PEP 489 multi-phase module initialization, including compatibility with the changes to persistent in zopefoundation/persistent#204. bit does not yet expect (or require) that PR to be merged.

Remove the local '_compat.h' header where they were defined, replacing
imports of it everwhere with 'Python.h'.

Note that the unrelated '_compat.h' header in the
'include/persistent/persistent/' directory is still in place.  No C
header or module imports it directly, and none uses any macros defined
in it indirectly.
@tseaver
Copy link
Member Author

tseaver commented Jun 3, 2024

I don't grok the Coveralls failures, but they appear to be complaining that master (not just this PRs branch) has dropped statement and / or branch coverage since the merge of PR #198.

@tseaver tseaver merged commit b792a20 into master Jun 3, 2024
52 of 54 checks passed
@tseaver tseaver deleted the tseaver-scrub_compat_fossils branch June 3, 2024 03:55
@davisagli
Copy link
Member

@tseaver It's not the first time I've seen confusing reports from coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants