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.

Sign up for GitHub

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

Jump to bottom

Refactor websockets dependencies: move the optional dependency to extras and unite imports #51

Merged
merged 3 commits into from
Jan 13, 2025
Merged

Refactor websockets dependencies: move the optional dependency to extras and unite imports #51

merged 3 commits into from
Jan 13, 2025

Conversation

mberdyshev
Copy link
Contributor

No description provided.

@mberdyshev mberdyshev force-pushed the reformat-optional-dependency branch from 9772e74 to ba107d8 Compare December 6, 2024 07:01
@mberdyshev mberdyshev force-pushed the reformat-optional-dependency branch from ba107d8 to 1be938c Compare December 6, 2024 07:06
@mberdyshev
Copy link
Contributor Author

mberdyshev commented Dec 6, 2024 •
edited
Loading

websockets dropped support of Python 3.8 and lower. Should the library drop it too? It has reached the EOL.
UPD: Dropped them.

@mberdyshev
Copy link
Contributor Author

mberdyshev commented Dec 12, 2024 •
edited
Loading

@orweis @danyi1212 Do you have time to take a look at it and review the code?

iwphonedo
iwphonedo reviewed Dec 23, 2024
View reviewed changes
Copy link

@iwphonedo iwphonedo left a comment

Choose a reason for hiding this comment

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

@mberdyshev I think the new build workflow is starting now with python 3.9 and on, try to pull from the last commit and run again.

@mberdyshev mberdyshev force-pushed the reformat-optional-dependency branch from 5ea399e to 6416c84 Compare December 23, 2024 22:01
iwphonedo
iwphonedo suggested changes Dec 24, 2024
View reviewed changes
Copy link

@iwphonedo iwphonedo left a comment

Choose a reason for hiding this comment

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

@mberdyshev thanks for this, I added my comments, and am happy to help with anything needed.

fastapi_websocket_rpc/websocket_rpc_client.py Show resolved Hide resolved
fastapi_websocket_rpc/websocket_rpc_client.py Show resolved Hide resolved
fastapi_websocket_rpc/websocket_rpc_client.py Show resolved Hide resolved
fastapi_websocket_rpc/websocket_rpc_client.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
tests/fast_api_depends_test.py Show resolved Hide resolved
@mberdyshev mberdyshev requested a review from iwphonedo December 26, 2024 06:49
iwphonedo
iwphonedo approved these changes Jan 1, 2025
View reviewed changes
Copy link

@iwphonedo iwphonedo left a comment

Choose a reason for hiding this comment

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

@mberdyshev This looks good to me know. Thank you!

@mberdyshev
Remove EOL Python versions. Support for Python >= 3.9
05b3301 
Support latest versions for tests.
@mberdyshev mberdyshev force-pushed the reformat-optional-dependency branch from 6416c84 to 05b3301 Compare January 8, 2025 12:10
@mberdyshev
Copy link
Contributor Author

@orweis, hello! Do you have time now for the code review?

@orweis
Copy link
Contributor

orweis commented Jan 10, 2025

I believe @iwphonedo is on the case.
Izzy, let us know if you can get to this soon.

@mberdyshev
Copy link
Contributor Author

@iwphonedo has already approved this PR. However, GitHub writes that merging is blocked because "At least 1 approving review is required by reviewers with write access".

orweis
orweis approved these changes Jan 12, 2025
View reviewed changes
Copy link
Contributor

@orweis orweis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Approved - @mberdyshev once you're ready you can merge / ask me to merge.
Thank you for your contribution.

@gemanor @danielbass37 - let's offer the man some Permit Swag as thank you

@mberdyshev
Copy link
Contributor Author

@orweis I have no available option to merge this PR, so you are welcome to do it.

@orweis orweis merged commit 919d791 into permitio:master Jan 13, 2025
7 checks passed
@orweis
Copy link
Contributor

orweis commented Jan 13, 2025

@orweis I have no available option to merge this PR, so you are welcome to do it.

Congratulations - it's merged 😇

@mberdyshev
Copy link
Contributor Author

mberdyshev commented Jan 13, 2025 •
edited
Loading

Thank you! Is it possible to make a new PyPI release?

@orweis
Copy link
Contributor

orweis commented Jan 13, 2025

Thank you! Is it possible to make a new PyPI release?

https://pypi.org/project/fastapi-websocket-rpc/0.1.28/ ;-)

@mberdyshev
Copy link
Contributor Author

Great! Thank you a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@obsd obsd obsd left review comments

@iwphonedo iwphonedo iwphonedo approved these changes

@orweis orweis orweis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mberdyshev @orweis @obsd @iwphonedo