Skip to content

Conversation

@rg0now
Copy link
Contributor

@rg0now rg0now commented Jan 5, 2026

Description

Wrap net.ListenConfig to allow customizing vnet Listeners.

Reference issue

Related pion/turn#514

@rg0now rg0now requested review from JoTurk and Sean-Der January 5, 2026 19:34
@codecov
Copy link

codecov bot commented Jan 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.25%. Comparing base (37ee413) to head (eb98700).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #373      +/-   ##
==========================================
- Coverage   84.34%   84.25%   -0.09%     
==========================================
  Files          41       41              
  Lines        3194     3208      +14     
==========================================
+ Hits         2694     2703       +9     
- Misses        359      363       +4     
- Partials      141      142       +1     
Flag Coverage Δ
go 84.25% <100.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoTurk
Copy link
Member

JoTurk commented Jan 5, 2026

@rg0now is it possible to fix the API compatibility? maybe we should keep transport.Net unchanged and introduce the option as an extension interface.

@rg0now
Copy link
Contributor Author

rg0now commented Jan 5, 2026

@JoTurk I agree API breakage would be best to avoid. Here is a typical point where we want to use this. The idea is that we create a custom net.ListenConfig with the required sockopts applied and then call ListenPacket on that, and we want to do this all via transport.Net. Without this change we have to hard-code net.ListenConfig.

How to make this work? Do you have a concrete suggestion? Happy to redo this

@Sean-Der
Copy link
Member

Sean-Der commented Jan 5, 2026

@JoTurk I think it might be ok! If we bump pion/transport it is just used internally by pion libraries. I don't think it should bother users too much.

Sorry if I put lots of anxiety on people around major version dumps. It's just a bummer that more people still use v3 vs v4. 2x more still import v3 and would love to fix that :'(

https://pkg.go.dev/github.com/pion/webrtc/v3
https://pkg.go.dev/github.com/pion/webrtc/v4

Copy link
Member

@JoTurk JoTurk left a comment

Choose a reason for hiding this comment

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

Yeah i was a semver absolute with my comment, there are work arounds but they are all seems to be ugly.

@Sean-Der
Copy link
Member

Sean-Der commented Jan 6, 2026

@rg0now @JoTurk 👍 for me you both ok with merging + new major version? I can make sure all the deps get bumped :)

@rg0now
Copy link
Contributor Author

rg0now commented Jan 8, 2026

@JoTurk @Sean-Der Please merge & bump version, I'm dangerously unreliable when it comes to release engineering

@JoTurk
Copy link
Member

JoTurk commented Jan 8, 2026

@rg0now Ok. I'm merging #370 and then i'm going to merge this and do major version bump, will go and upgrade it everywhere :)

@JoTurk JoTurk merged commit db239a8 into pion:master Jan 8, 2026
17 of 18 checks passed
@JoTurk
Copy link
Member

JoTurk commented Jan 8, 2026

@rg0now @Sean-Der we have few soft deprecated apis, since we're doing a major bump I'm going to open a PR right now to remove them.

@JoTurk
Copy link
Member

JoTurk commented Jan 8, 2026

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