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

Break out internal/net/udp #267

Merged
merged 1 commit into from
Jun 21, 2020
Merged

Break out internal/net/udp #267

merged 1 commit into from
Jun 21, 2020

Conversation

backkem
Copy link
Member

@backkem backkem commented Jun 20, 2020

Breaking out the UDP wrapper to make it re-
usable in pion/sctp.

Relates to pion/sctp#74

TODO:

  • Tag pion/udp after landing CI

@backkem backkem requested a review from at-wat June 20, 2020 19:47
@codecov
Copy link

codecov bot commented Jun 20, 2020

Codecov Report

Merging #267 into master will decrease coverage by 0.44%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
- Coverage   77.82%   77.38%   -0.45%     
==========================================
  Files          76       75       -1     
  Lines        4077     3944     -133     
==========================================
- Hits         3173     3052     -121     
+ Misses        623      611      -12     
  Partials      281      281              
Flag Coverage Δ
#go 77.42% <ø> (-0.45%) ⬇️
#wasm 75.07% <ø> (+2.42%) ⬆️
Impacted Files Coverage Δ
listener.go 45.71% <ø> (ø)
internal/net/connctx/connctx.go 86.41% <0.00%> (-2.47%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f813a7d...daba2c5. Read the comment docs.

@backkem backkem requested a review from Sean-Der June 20, 2020 20:32
go.mod Outdated
@@ -3,6 +3,7 @@ module github.com/pion/dtls/v2
require (
github.com/pion/logging v0.2.2
github.com/pion/transport v0.10.0
github.com/pion/udp v0.0.0-20200620185302-adf622c7c621
Copy link
Member

Choose a reason for hiding this comment

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

Mind tagging v0.0.1

Copy link
Member

Choose a reason for hiding this comment

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

Never mind you already made a note :)

@Sean-Der
Copy link
Member

Are you able to delete github.com/pion/dtls/v2/internal/net/udp now?

Copy link
Member

@at-wat at-wat left a comment

Choose a reason for hiding this comment

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

I definitely agree to split udp package and make it exposed. (pion/transport#57)
Do you think it's better to be a independent package out of pion/transport?

@backkem
Copy link
Member Author

backkem commented Jun 21, 2020

I definitely agree to split udp package and make it exposed. (pion/transport#57)
Do you think it's better to be a independent package out of pion/transport?

I think we've had overall positive experience with putting 'protocols' in their own repo. While pion/udp is only a thin wrapper it may still be useful on its own. In such case I prefer a separate repo since I want to encourage others to mix and match our protocols into their own stack. The DTLS implementation has been a prime example of this.
pion/transport is becoming a bit of a 'utils' package. That's fine for tooling we use across our libraries. However, if some piece, like maybe the mux, are useful to let people throw together their own stack, I'd consider exposing them using their own repo.

Breaking out the UDP wrapper to make it re-
usable in pion/sctp.

Relates to pion/sctp#74
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.

3 participants