GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-109413: Enable `strict_optional = true` for `libregrtest/run_workers` by sobolevn · Pull Request #126855 · python/cpython

Conversation

@sobolevn

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: libregrtest: Improve type hints; explore running mypy in CI #109413

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn
pythongh-109413: Enable strict_optional = true for `libregrtest/run…
7c5f178 
…_workers`
@sobolevn
sobolevn requested review from AlexWaygood and vstinner November 15, 2024 08:23
@bedevere-app bedevere-app Bot added the awaiting core review label Nov 15, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Nov 15, 2024
libregrtest: Improve type hints; explore running mypy in CI #109413
Closed
@sobolevn sobolevn added skip news needs backport to 3.13 only security fixes labels Nov 15, 2024
AlexWaygood
AlexWaygood approved these changes Nov 15, 2024
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks great, thank you!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Nov 15, 2024
@sobolevn
Address review
21fd256
vstinner
vstinner reviewed Nov 15, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn
Address review
304065d
vstinner
vstinner approved these changes Nov 15, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn

sobolevn commented Nov 15, 2024

Copy link
Copy Markdown
Member Author

Failure is related:

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

Fixing
AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn
Fix CI
9475401
AlexWaygood
AlexWaygood reviewed Nov 15, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn @AlexWaygood
Apply suggestions from code review
a4e1071 
Co-authored-by: Alex Waygood <[email protected]>
AlexWaygood
AlexWaygood approved these changes Nov 15, 2024
View reviewed changes
vstinner
vstinner reviewed Nov 15, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn
Address review
ed9a8e3
vstinner
vstinner reviewed Nov 18, 2024
View reviewed changes
Comment thread Lib/test/libregrtest/run_workers.py
super().__init__()


_NOT_RUNNING = "<not running>"

@vstinner vstinner Nov 18, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood reacted with laugh emoji
All reactions
  • 😄 1 reaction

@sobolevn sobolevn Nov 18, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

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 :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn
Address review
b2d6a89
vstinner
vstinner approved these changes Nov 18, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/libregrtest/run_workers.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn
Address review
676e426
@sobolevn
sobolevn merged commit a1d9c8a into python:main Nov 18, 2024
@miss-islington-app

miss-islington-app Bot commented Nov 18, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Nov 18, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 18, 2024
@sobolevn @AlexWaygood @miss-islington
pythongh-109413: Enable strict_optional = true for `libregrtest/run…
e505685 
…_workers` (pythonGH-126855)

(cherry picked from commit a1d9c8a)

Co-authored-by: sobolevn <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
@bedevere-app

bedevere-app Bot commented Nov 18, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Nov 18, 2024
@sobolevn

sobolevn commented Nov 18, 2024

Copy link
Copy Markdown
Member Author

Thanks everyone for so many helpful suggestions! It turned out very good! 👍
vstinner reacted with thumbs up emoji AlexWaygood reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Nov 18, 2024

Copy link
Copy Markdown
Member

Thanks for your nice change.
sobolevn reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sobolevn added a commit that referenced this pull request Nov 18, 2024
@miss-islington @sobolevn @AlexWaygood
[3.13] gh-109413: Enable strict_optional = true for `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 <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
@colesbury colesbury mentioned this pull request Nov 22, 2024
<not running> console spam when running tests #127152
Closed
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@sobolevn @AlexWaygood @ebonnal
pythongh-109413: Enable strict_optional = true for `libregrtest/run…
d8ce082 
…_workers` (python#126855)

Co-authored-by: Alex Waygood <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@AlexWaygood AlexWaygood AlexWaygood approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sobolevn @vstinner @AlexWaygood
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.