GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||
GeezerVison is showing github.com — Many 'o' fixes by ntninja · Pull Request #71 · python-trio/unasync ConversationMany fixes for:
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…hon` (b/c it is named `python3` only like in Debian) …coverage when running on each Implementation: Detect type comments by their `# type: ` prefix, recursively unasyncify the type declaration part as new token stream, update the otherwise unchanged token value.
| |||||||||||||||||||||||
There was an error while loading. Please reload this page.
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Can you make the coverage reports public please? It cannot view them and I don't see any coverage misses when running locally. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @@ -0,0 +1,10 @@ | |||
| [run] | |||
| branch=True | |||
| source=unasync | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This might be causing a problem for Python 2 coverage, might need a [paths] source = src/unasync entry too.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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.
Hmm… That worked, then stopped working, then started working again. 😕
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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.) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.