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

Remove use of six.ensure_str/ensure_text in wptrunner #28935

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

foolip
Copy link
Member

@foolip foolip commented May 10, 2021

Part of #28776.

@foolip
Copy link
Member Author

foolip commented May 10, 2021

I intend to leave this until #28942 is done, to be able to type check the results a bit.

@foolip foolip force-pushed the foolip/six_wptrunner branch 2 times, most recently from 2afcaa3 to 90af724 Compare March 31, 2022 15:04
@foolip foolip force-pushed the foolip/six_wptrunner branch from 90af724 to 087c8ac Compare April 1, 2022 10:55
@foolip
Copy link
Member Author

foolip commented Apr 5, 2022

@gsnedders @jgraham these are our last direct uses of six. I had a hard time convincing myself that the type was guaranteed to be str at the call sites, so I made variants of ensure_str and ensure_text that would exit the process if that wasn't the case. Is CI passing with this change enough to convince you that we can just assume str, or do you think there's more due diligence that should be done here?

@foolip
Copy link
Member Author

foolip commented Mar 21, 2023

Ping @gsnedders and @jgraham.

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.

1 participant