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

Small cleanups to IP connection #338

Merged
merged 5 commits into from
Oct 19, 2023
Merged

Small cleanups to IP connection #338

merged 5 commits into from
Oct 19, 2023

Conversation

bdraco
Copy link
Collaborator

@bdraco bdraco commented Oct 19, 2023

  • Add more typing
  • Add some doc strings
  • Avoid async_timeout overhead when waiting for a future

@bdraco
Small cleanups to IP connection
4e9b07b 
- Add more typing
- Add some doc strings
- Avoid async_timeout overhead when waiting for a future
@codecov
Copy link

codecov bot commented Oct 19, 2023 •
edited
Loading

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (c244405) 67.99% compared to head (4e9b07b) 67.99%.

❗ Current head 4e9b07b differs from pull request most recent head b7249b9. Consider uploading reports for the commit b7249b9 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
- Coverage   67.99%   67.99%   -0.01%     
==========================================
  Files          71       71              
  Lines        6781     6789       +8     
==========================================
+ Hits         4611     4616       +5     
- Misses       2170     2173       +3
Files Coverage Δ
aiohomekit/controller/ip/connection.py 81.00% <90.00%> (-0.46%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Jc2k
Jc2k reviewed Oct 19, 2023
View reviewed changes
Copy link
Owner

@Jc2k Jc2k 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.

Body is bytes|None (this was meant to be against request() but I apparently tapped on the wrong pixels)

Jc2k
Jc2k requested changes Oct 19, 2023
View reviewed changes
Copy link
Owner

@Jc2k Jc2k 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.

Looks good! Thanks. I think we can make body types tighter but otherwise all good

aiohomekit/controller/ip/connection.py Outdated Show resolved Hide resolved
aiohomekit/controller/ip/connection.py Outdated Show resolved Hide resolved
@bdraco bdraco marked this pull request as ready for review October 19, 2023 08:51
@bdraco bdraco requested a review from Jc2k October 19, 2023 08:51
Jc2k
Jc2k approved these changes Oct 19, 2023
View reviewed changes
Copy link
Owner

@Jc2k Jc2k left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks. I think we can make body types tighter but otherwise all good

@bdraco
Copy link
Collaborator Author

bdraco commented Oct 19, 2023

Ran overnight all good 👍

@bdraco bdraco merged commit b3e6f61 into main Oct 19, 2023
7 checks passed
@bdraco bdraco deleted the connection_cleanups branch October 19, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jc2k Jc2k Jc2k 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.
2 participants
@bdraco @Jc2k