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

Update modules structure and other minor improvements for Rust version #558

Merged
merged 9 commits into from
May 19, 2021

Conversation

nazar-pc
Copy link
Collaborator

@nazar-pc nazar-pc commented May 15, 2021

Changes:

  • Refactored modules structure: it still looks the same to developers and docs, but doesn't confuse IDE as much so autocomplete works well
  • NonClosingProducer renamed to PipeProducer with better documentation about why it exists (old name is still available)
  • Minor fix in libmediasoup-worker that avoids printing irrelevant messages

I will update changelog and bump versions in upcoming PRs once macOS support is merged upstream (hopefully very soon).

Copy link
Member

@jmillan jmillan left a comment

Choose a reason for hiding this comment

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

I don't know Rust myself, but the changes seem to enhance readability and inline documentation.

Considering tests pass I'm OK with the PR.

@nazar-pc
Copy link
Collaborator Author

Pushed an update to fix some of the lints.

Do you plan to update libwebrtc to fix numeric_limits issues in CI with Ubuntu 18.04/20.04 and Clang (if it does fix it)?
Looks like an import is missing of something like that.

@jmillan
Copy link
Member

jmillan commented May 17, 2021

Do you plan to update libwebrtc to fix numeric_limits issues in CI with Ubuntu 18.04/20.04 and Clang (if it does fix it)?

The problem is within the abseil dependency used by libwebrtc. I Don't think we want to clone it just to avoid this issue, since also there are being some efforts to remove libwebrtc completely from mediasoup.

Copy link
Member

@ibc ibc left a comment

Choose a reason for hiding this comment

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

I don't see any bad words in the changes so I assume it's ok. Merging.

@ibc ibc merged commit 8a301b0 into versatica:v3 May 19, 2021
@nazar-pc nazar-pc deleted the rust-next branch May 19, 2021 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants