Skip to content

[59]:.. Bindings for reuseaddr #63

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
Merged
merged 9 commits into from
Mar 23, 2023
Merged

[59]:.. Bindings for reuseaddr #63

merged 9 commits into from
Mar 23, 2023

Conversation

Darrellbor
Copy link
Contributor

PR for #59
Implemented:

  • binding for roc_receiver_set_reuseaddr
  • binding for roc_sender_set_reuseaddr
  • test for sender.SetReuseaddr()
  • test for receiver.SetReuseaddr()

@coveralls
Copy link
Collaborator

coveralls commented Mar 21, 2023 •
edited
Loading

Coverage Status

Coverage: 77.127% (+2.3%) from 74.847% when pulling e412a32 on Darrellbor:50-bindings_for_reuseaddr into edd5296 on roc-streaming:main.

ortex
ortex requested changes Mar 22, 2023
View reviewed changes
@Darrellbor
feat: PR feedback: assertion correction, move sender specific test to…
08f4323 
… it's own file, use enabled as bool instead of int
@Darrellbor
Copy link
Contributor Author

pushed the fixes, although the tests stopped, can you restart it

@ortex
Copy link
Member

ortex commented Mar 22, 2023

please, rebase on fresh main

gavv
gavv reviewed Mar 22, 2023
View reviewed changes
Copy link
Member

@gavv gavv left a comment

Choose a reason for hiding this comment

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

Thanks for PR! A few comments from me too

@gavv
Copy link
Member

gavv commented Mar 22, 2023

BTW when request is ready for (re-)review, please leave a comment or press "request review" button to indicate that - otherwise it's hard to guess whether it's already time to look at the pushed changes.

@Darrellbor
Copy link
Contributor Author

Darrellbor commented Mar 22, 2023 •
edited
Loading

Noted @gavv
ready for re-review

@Darrellbor Darrellbor requested review from gavv and ortex and removed request for gavv and ortex March 22, 2023 21:26
gavv
gavv reviewed Mar 23, 2023
View reviewed changes
@gavv
Copy link
Member

gavv commented Mar 23, 2023 •
edited
Loading

Thanks for update!

+1 from me.

@ortex could you take a look?

@gavv gavv added the status: ready for review Pull request can be reviewed label Mar 23, 2023
@ortex
Copy link
Member

ortex commented Mar 23, 2023

LGTM

@Darrellbor Darrellbor requested a review from gavv March 23, 2023 12:37
@gavv gavv merged commit ee4158d into roc-streaming:main Mar 23, 2023
@gavv
Copy link
Member

gavv commented Mar 23, 2023

Thank you!

@gavv gavv removed the status: ready for review Pull request can be reviewed label Mar 23, 2023
@Darrellbor Darrellbor deleted the 50-bindings_for_reuseaddr branch March 23, 2023 12:48
@gavv
Copy link
Member

gavv commented Mar 23, 2023

Welcome to matrix chat BTW :) https://roc-streaming.org/toolkit/docs/about_project/contacts.html

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

@ortex ortex ortex approved these changes

@gavv gavv Awaiting requested review from gavv

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
@Darrellbor @coveralls @ortex @gavv