← 返回首页
Many 'o' fixes by ntninja · Pull Request #71 · python-trio/unasync · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

Many 'o' fixes#71

Open
ntninja wants to merge 8 commits into
python-trio:masterfrom
ntninja:master
Open

Many 'o' fixes#71
ntninja wants to merge 8 commits into
python-trio:masterfrom
ntninja:master

Conversation

Copy link
Copy Markdown

ntninja commented Dec 18, 2020

Many fixes for:

  • Running tests on Debian/Linux
  • Coverage reporting wrt Python 2 & 3 only code
  • Source code indented with tabs (like all of mine)
  • Type comments (old-style typings for compatiblity with Python 2.7 and, in some cases, 3.5)
  • Forward-references (string values in typings)
  • pathlib.Path/os.fspath filesystem paths

…hon` (b/c it is named `python3` only like in Debian)
Implementation: Detect type comments by their `# type: ` prefix, recursively unasyncify the type declaration part as new token stream, update the otherwise unchanged token value.
Copy link
Copy Markdown

codecov Bot commented Dec 18, 2020
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9b15d0e) to head (3699165).
⚠️ Report is 21 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@ ## master #71 +/- ## ========================================== Coverage 100.00% 100.00% ========================================== Files 4 4 Lines 254 424 +170 Branches 62 120 +58 ========================================== + Hits 254 424 +170
Files with missing lines Coverage Δ
src/unasync/__init__.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Author

ntninja commented Dec 18, 2020

Can you make the coverage reports public please? It cannot view them and I don't see any coverage misses when running locally.

Copy link
Copy Markdown
Contributor

sethmlarson left a comment

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Thanks for this!

A high level comment is it might be easier to review all of this if it were split into multiple smaller PRs. Would you be able to do that?

It looks like there was a problem shipping the coverage data? Our reports should be public as far as I'm aware? https://codecov.io/gh/python-trio/unasync/pull/71

Comment thread .coveragerc-py2
@@ -0,0 +1,10 @@
[run]
branch=True
source=unasync
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

This might be causing a problem for Python 2 coverage, might need a [paths] source = src/unasync entry too.

Implementation: A context tracker monitors the incomping token stream and reports whether we are currently in a typing context or not, if we are in a typing context then the normal replacement code treats string values as a new token string to unasyncify recursively.
Copy link
Copy Markdown
Author

ntninja commented Dec 22, 2020

A high level comment is it might be easier to review all of this if it were split into multiple smaller PRs. Would you be able to do that?

I'd strongly prefer not doing that, as the commits build on each other and separating them into different PRs would be a big mess. That said, each commit is self-contained and you can review them one-by-one on Github.

It looks like there was a problem shipping the coverage data? Our reports should be public as far as I'm aware? https://codecov.io/gh/python-trio/unasync/pull/71

Hmm… That worked, then stopped working, then started working again. 😕

This might be causing a problem for Python 2 coverage, might need a [paths] source = src/unasync entry too.

I tried this in both coverage files and it did nothing. However, one can see the actual summary coverage data on CodeCov by clicking on the Files and then on the src/unasync directory which appears to be an exact mirror of the unasync directory in terms of coverage data, but is actually able to load the data.

Copy link
Copy Markdown
Member

I still need to review two commits: 600807e and 4527a95. The rest looks good to me!

(Sorry, it took me time to see this as I was on holidays and GitHub notifications only showed me recent items.)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants

Footer

© 2026 GitHub, Inc.