Skip to content

gh-109413: Enable strict_optional = true for libregrtest/run_workers - #126855

Merged
sobolevn merged 8 commits into
python:mainfrom
sobolevn:issue-109413-run_workers
Nov 18, 2024
Merged

gh-109413: Enable strict_optional = true for libregrtest/run_workers#126855
sobolevn merged 8 commits into
python:mainfrom
sobolevn:issue-109413-run_workers

Conversation

@sobolevn

@sobolevnsobolevn commented Nov 15, 2024•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-appbedevere-appBot mentioned this pull request Nov 15, 2024
Closed
AlexWaygood
AlexWaygood approved these changes Nov 15, 2024
View reviewed changes

@AlexWaygoodAlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great, thank you!

Comment threadLib/test/libregrtest/run_workers.py
vstinner
vstinner reviewed Nov 15, 2024
View reviewed changes
Comment threadLib/test/libregrtest/run_workers.py Outdated
vstinner
vstinner approved these changes Nov 15, 2024
View reviewed changes

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@sobolevn

sobolevn commented Nov 15, 2024

Copy link
Copy Markdown
MemberAuthor

Failure is related:

 raise ValueError(
'Should never call `.test_name` before calling `.run()`'
)
ValueError: Should never call `.test_name` before calling `.run()`

Fixing

AlexWaygood
AlexWaygood reviewed Nov 15, 2024
View reviewed changes
Comment threadLib/test/libregrtest/run_workers.py Outdated
Comment threadLib/test/libregrtest/run_workers.py Outdated
@sobolevn@AlexWaygood
Apply suggestions from code review
a4e1071
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
vstinner
vstinner reviewed Nov 15, 2024
View reviewed changes
Comment threadLib/test/libregrtest/run_workers.py Outdated
Comment threadLib/test/libregrtest/run_workers.py Outdated
vstinner
vstinner reviewed Nov 18, 2024
View reviewed changes
Comment threadLib/test/libregrtest/run_workers.py
super().__init__()


_NOT_RUNNING = "<not running>"

@vstinnervstinnerNov 18, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can make it public, it's not a deep secret.

@sobolevnsobolevnNov 18, 2024•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think that it needs to be public, since it is not used outside of this module and probably won't be. If you prefer to have it as public - I will change it :)

Comment threadLib/test/libregrtest/run_workers.py Outdated
Comment threadLib/test/libregrtest/run_workers.py Outdated
Comment threadLib/test/libregrtest/run_workers.py Outdated
Comment threadLib/test/libregrtest/run_workers.py Outdated
vstinner
vstinner approved these changes Nov 18, 2024
View reviewed changes

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment threadLib/test/libregrtest/run_workers.py Outdated
@sobolevn
sobolevn merged commit a1d9c8a into python:mainNov 18, 2024
@miss-islington-app

miss-islington-appBot commented Nov 18, 2024

Copy link
Copy Markdown

Thanks @sobolevn for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 18, 2024
@sobolevn@AlexWaygood@miss-islington
pythongh-109413: Enablestrict_optional = truefor `libregrtest/run…
e505685
…_workers` (pythonGH-126855)
(cherry picked from commit a1d9c8a)
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@bedevere-app

bedevere-appBot commented Nov 18, 2024

Copy link
Copy Markdown

GH-126967 is a backport of this pull request to the 3.13 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Nov 18, 2024
@sobolevn

sobolevn commented Nov 18, 2024

Copy link
Copy Markdown
MemberAuthor

Thanks everyone for so many helpful suggestions! It turned out very good! 👍

@vstinner

vstinner commented Nov 18, 2024

Copy link
Copy Markdown
Member

Thanks for your nice change.

sobolevn added a commit that referenced this pull request Nov 18, 2024
@miss-islington@sobolevn@AlexWaygood
[3.13]gh-109413: Enablestrict_optional = truefor `libregrtest/ru…
a7b44d6
…n_workers` (GH-126855) (#126967)
gh-109413: Enable `strict_optional = true` for `libregrtest/run_workers` (GH-126855)
(cherry picked from commit a1d9c8a)
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@colesburycolesbury mentioned this pull request Nov 22, 2024
Closed
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@sobolevn@AlexWaygood@ebonnal
pythongh-109413: Enablestrict_optional = truefor `libregrtest/run…
d8ce082
…_workers` (python#126855)
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinnervstinnervstinner approved these changes
@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sobolevn@vstinner@AlexWaygood