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

Run mypy for tools/serve/ (but still allow untyped defs) #28988

Merged
merged 1 commit into from
Jun 13, 2021

Conversation

foolip
Copy link
Member

@foolip foolip commented May 13, 2021

Part of #28833.

tools/serve/serve.py Outdated Show resolved Hide resolved
@foolip foolip force-pushed the foolip/mypy-serve branch 3 times, most recently from 607d4da to 349eb96 Compare May 18, 2021 13:11
@foolip foolip marked this pull request as ready for review June 11, 2021 13:35
@wpt-pr-bot wpt-pr-bot requested a review from jgraham June 11, 2021 13:35
@foolip foolip removed their assignment Jun 11, 2021

from localpaths import repo_root
from localpaths import repo_root # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the error here? Seems like we should be able to mark this as a str.

Copy link
Member Author

Choose a reason for hiding this comment

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

The error is "Cannot find implementation or library stub for module named 'localpaths'"

This kind of thing has come up in many places, and I'm thinking that perhaps together with web-platform-tests/rfcs#82 we could think of a path to having no sys.path meddling and having all imports resolve correctly for mypy, but I'm not sure now what it would entail.

Does that sound alarming, or does your review stand?

@jgraham jgraham merged commit 9cbcc97 into master Jun 13, 2021
@jgraham jgraham deleted the foolip/mypy-serve branch June 13, 2021 13:06
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