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

Please revert switch to v2 #129

Closed
jech opened this issue May 17, 2021 · 6 comments
Closed

Please revert switch to v2 #129

jech opened this issue May 17, 2021 · 6 comments

Comments

@jech
Copy link
Member

jech commented May 17, 2021

Commit 863e835 updates the package version to v2. This causes a lot of churn for no good reason, since commit 6b516fb merely removes some fields that nobody is using anyhow (that's why they're being removed).

An alternative would be to revert both commits.

If reverting this commit is not possible, then we need an update to pion/webrtc ASAP, since the samplebuilder takes a v1 packet.

@aler9
Copy link
Member

aler9 commented May 22, 2021

I added the v2 version since:

  • The removal of deprecated fields (PayloadOffset and Raw in this case) is one of the reasons listed in the Golang blog to perform an update to a major version, doesn't matter if the fields are seldom used

  • The author clearly stated that changes are part of a v2 plan

If there's the need of sticking to a v1 for the time being (since the main maintainer is currently unavailable), an alternative solution consists in moving v2 changes into a dedicated branch.

aler9 added a commit that referenced this issue May 22, 2021
Since /v2 is causing interaction problems with other parts of the
Pion project, i'm removing my previous commit. The issue must
anyway dealt with in a future discussion with the community.

Fixes #129
@aler9 aler9 mentioned this issue May 22, 2021
@at-wat
Copy link
Member

at-wat commented Jul 8, 2021

In my personal opinion, it's better not to hesitate to bump major version if necessary.
But, if there are other possible breaking changes, it would be better to do them at once before bumping the major version.

@jech
Copy link
Member Author

jech commented Jul 8, 2021

@at-wat I agree. The problem here is that switching to v2 put pion/rtc and pion/webrtc out of sync — it was no longer possible to link a recent webrtc against a recent rtp, which forced some git gymnastics if you wanted to tweak rtc. Just like you, I have no objection to major version changes if they are necessary and all dependencies are updated in a timely manner.

@Sean-Der
Copy link
Member

Sean-Der commented Jul 9, 2021

If we upgraded webrtc, srtp and interceptor to github.com/pion/rtp/v2 would that unblock you @jech?

I am fine doing the work to make that upgrade! Would rather move forward the undo contributions.

@jech
Copy link
Member Author

jech commented Jul 9, 2021

This one is not blocking me right now, it just prevents me from contributing to pion/rtp: since any contribution I make will be done to a version that I cannot use, I'd need to maintain my own private branch and do some creative replacing in the modules, which is just not worth the hassle.

The one that's a hard blocker is pion/webrtc#1874, where my bug fixes got into a version that I cannot use due to a broken samplebuilder. The only solutions I can see is to maintain my own samplebuilder that's not broken, or to stop using the samplebuilder. This has been broken for over a month not, so it's not just a temporary fluke.

@Sean-Der
Copy link
Member

Sean-Der commented Aug 2, 2021

I created a v0 branch and will be cherry-picking changes for now.

I attempted to upgrade pion/webrtc to pion/rtp@v2 but it would be a breaking change. So we need to wait until pion/webrtc@v4

@Sean-Der Sean-Der closed this as completed Aug 2, 2021
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 a pull request may close this issue.

4 participants