Skip to content

bpo-42243: Use PyModule_Add*() in _sre iso. direct module dict manipulation - #23101

Closed
erlend-aasland wants to merge 3 commits into
python:masterfrom
erlend-aasland:bpo-42243/sre
Closed

bpo-42243: Use PyModule_Add*() in _sre iso. direct module dict manipulation#23101
erlend-aasland wants to merge 3 commits into
python:masterfrom
erlend-aasland:bpo-42243/sre

Conversation

@erlend-aasland

@erlend-aaslanderlend-aasland commented Nov 2, 2020•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

serhiy-storchaka
serhiy-storchaka reviewed Nov 2, 2020
View reviewed changes
Comment threadModules/_sre.c Outdated
Comment threadModules/_sre.c Outdated
@serhiy-storchaka

serhiy-storchaka commented Nov 2, 2020

Copy link
Copy Markdown
Member

Just for the case, add tests that MAXREPEAT and MAXGROUPS > 0.

@erlend-aasland
erlend-aasland marked this pull request as draft November 2, 2020 09:20
@erlend-aasland
erlend-aasland marked this pull request as ready for review November 2, 2020 10:30
@erlend-aasland

erlend-aasland commented Nov 2, 2020•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Just for the case, add tests that MAXREPEAT and MAXGROUPS > 0.

Would a simple assert() be sufficient, or did you mean adding a unit test?

@serhiy-storchaka

serhiy-storchaka commented Nov 3, 2020

Copy link
Copy Markdown
Member

Would a simple assert() be sufficient, or did you mean adding a unit test?

I meant adding a test in Lib/test/test_re.py. If somebody will break this code in future it will get clearer test failure.

@erlend-aasland

erlend-aasland commented Nov 3, 2020

Copy link
Copy Markdown
ContributorAuthor

Would a simple assert() be sufficient, or did you mean adding a unit test?

I meant adding a test in Lib/test/test_re.py. If somebody will break this code in future it will get clearer test failure.

Got it. Signedness test added in 8561628.

@erlend-aasland

erlend-aasland commented Nov 17, 2020

Copy link
Copy Markdown
ContributorAuthor

@serhiy-storchaka are you ok with this PR as it stands?

@erlend-aaslanderlend-aasland mentioned this pull request Nov 19, 2020
Merged
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Nov 19, 2020
Erlend E. Aasland
MergepythonGH-23101
12bf771
@erlend-aasland

erlend-aasland commented Nov 20, 2020

Copy link
Copy Markdown
ContributorAuthor

Closing, as this was incorporated in #23393

@erlend-aasland
erlend-aasland deleted the bpo-42243/sre branch December 26, 2020 19:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaAwaiting requested review from serhiy-storchaka

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@erlend-aasland@serhiy-storchaka@the-knights-who-say-ni@bedevere-bot