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 DebugMiddleware #1697

Merged
merged 1 commit into from Oct 7, 2022
Merged

Conversation

edmorley
Copy link
Contributor

@edmorley edmorley commented Oct 7, 2022

Since it's unused and untested as of #1640.

@Kludex
Copy link
Sponsor Member

Kludex commented Oct 7, 2022

Would you mind bumping a bit the coverage fail-under value? It should be enough the number that was on that PR before 👀 🙏

@edmorley
Copy link
Contributor Author

edmorley commented Oct 7, 2022

Yeah I was going to wait until the runs have finished to see what value I can use, since I suspect the exact coverage percentage might still be slightly different (🥔s! 😄).

@edmorley edmorley marked this pull request as ready for review October 7, 2022 13:03
@edmorley
Copy link
Contributor Author

edmorley commented Oct 7, 2022

I checked all runs and lowered the coverage percentage to 97.71, however on the next test run the Python 3.7 macos-latest job then resulted in a lower coverage percentage than it had before? Is the coverage percentage flaky?

This run said 97.69:
https://github.com/encode/uvicorn/actions/runs/3204878159/jobs/5236700388#step:8:25

Whereas the prior one said 97.71:
https://github.com/encode/uvicorn/actions/runs/3204794323/jobs/5236511876#step:8:25

@Kludex
Copy link
Sponsor Member

Kludex commented Oct 7, 2022

Is the coverage percentage flaky?

There's a chance that some test is flaky resulting in the coverage being as well... 👀

Something around files, and filenames on MacOS.

@edmorley
Copy link
Contributor Author

edmorley commented Oct 7, 2022

Should I return the coverage percentage back to what it was before this PR, or would you like to rerun the CI jobs to see if they pass next time? (I don't have permissions to retrigger CI)

@Kludex
Copy link
Sponsor Member

Kludex commented Oct 7, 2022

Return the coverage, please 🙏

Since it's unused and untested as of encode#1640.
@edmorley
Copy link
Contributor Author

edmorley commented Oct 7, 2022

Done :-)

@Kludex
Copy link
Sponsor Member

Kludex commented Oct 7, 2022

Thank you @edmorley 🎉

Btw, how did you find about this? 👀

@Kludex Kludex merged commit bfc8fb3 into encode:master Oct 7, 2022
@edmorley edmorley deleted the rm-debug-middleware branch October 7, 2022 14:07
@edmorley
Copy link
Contributor Author

edmorley commented Oct 7, 2022

I just happened to be looking at recent PRs and was wondering what the DebugMiddleware did, and so was looking for the file removal diff in the PR to see its original contents, only to see there was no such removal in that PR :-)

@Kludex
Copy link
Sponsor Member

Kludex commented Oct 7, 2022

Cool. Thank you! 🙏

Kludex pushed a commit to sephioh/uvicorn that referenced this pull request Oct 29, 2022
Kludex pushed a commit that referenced this pull request Oct 29, 2022
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.

None yet

2 participants