Skip to content

gh-128438: Use EnvironmentVarGuard for datetimetester.py to manage environment varibales #130002

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

Merged
merged 7 commits into from
Mar 27, 2025

Conversation

Wulian233
Copy link
Contributor

@Wulian233 Wulian233 commented Feb 11, 2025

It's easier to use this to manage environment variables

is part of #128439 (comment)

@Wulian233
Copy link
Contributor Author

Wulian233 commented Feb 11, 2025

Unrelated: I noticed that the github number just broke through 13xxxx! 😄
#130000

edit: https://github.com/python/cpython/actions/runs/13262366718/job/37021689070?pr=130002 The test failure has unlated with this

Request review @Eclips4 sorry to bother. Had a similar change before #129193. Thank you very much!

Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

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

Overall this is good, but I think the details need to be tweaked.

@bedevere-app
Copy link

bedevere-app bot commented Feb 18, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@Wulian233
Copy link
Contributor Author

Wulian233 commented Feb 22, 2025

I have made the requested changes; please review again

Used context manager.

edit: Also need "skip news" label, label check action failed

@bedevere-app
Copy link

bedevere-app bot commented Feb 22, 2025

Thanks for making the requested changes!

@pganssle, @Eclips4: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested review from Eclips4 and pganssle February 22, 2025 05:36
@Wulian233
Copy link
Contributor Author

@pganssle @Eclips4 ping~

Could we ready to merge this?

@Eclips4 Eclips4 merged commit a096a41 into python:main Mar 27, 2025
38 checks passed
@Eclips4
Copy link
Member

Eclips4 commented Mar 27, 2025

Thank you @Wulian233! Sorry for the waiting.

@Wulian233 Wulian233 deleted the datetimetester branch March 27, 2025 22:27
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 1, 2025
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants