Skip to content

Conversation

@Archmonger
Copy link
Contributor

@Archmonger Archmonger commented Feb 27, 2023

This PR makes idom.run utilize the same code as uvicorn.run in order to prevent unexpected exceptions.

See Kludex/uvicorn#1845 for details

Checklist

Please update this checklist as you complete each item:

  • Tests have been included for all bug fixes or added functionality.
  • The changelog.rst has been updated with any significant changes.
  • GitHub Issues which may be closed by this Pull Request have been linked.

@Archmonger
Copy link
Contributor Author

The fix nox commit doesn't seem to have fixed nox.

On a tangent, we should probably set main as a protected branch to prevent direct commits.

@rmorshea
Copy link
Collaborator

rmorshea commented Mar 2, 2023

Yeah, my bad. I deleted the old ReactPy releases not realizing that once I deleted the last one the name would become available again so was stressing trying to get a release out ASAP.

@rmorshea
Copy link
Collaborator

rmorshea commented Mar 2, 2023

Also, main is protected. It's just that, as the owner of the repo I'm able to bypass those restrictions.

@Archmonger
Copy link
Contributor Author

@rmorshea Do you agree with removing VictoryBar tests due to our module_from_template deprecation? That test is super flaky anyways.

@rmorshea
Copy link
Collaborator

rmorshea commented Mar 4, 2023

If we remove the tests we should probably just remove the feature entirely.

@Archmonger
Copy link
Contributor Author

Archmonger commented Apr 4, 2023

I don't feel like this PR was appropriate to remove module_from_template, so I backed those changes out.

@Archmonger Archmonger marked this pull request as ready for review April 4, 2023 22:51
@Archmonger Archmonger requested a review from rmorshea April 5, 2023 01:28
@Archmonger Archmonger merged commit 8caf973 into reactive-python:main Apr 5, 2023
@Archmonger Archmonger deleted the fix-idom-run-uvicorn branch April 5, 2023 02:24
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.

2 participants