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

feat: set/create events #71

Merged
merged 9 commits into from
Feb 14, 2024
Merged

feat: set/create events #71

merged 9 commits into from
Feb 14, 2024

Conversation

SomeBody16
Copy link

@SomeBody16 SomeBody16 commented Feb 8, 2024

Added events that will be called on success operations on offer/answer/description

This includes

  • CreateOfferOnSuccess - when createOffer succeeded
  • CreateAnswerOnSuccess - when createAnswer succeeded
  • SetLocalDescriptionOnSuccess - when setLocalDescription succeeded
  • SetRemoteDescriptionOnSuccess - when setRemoteDescription succeeded

@SomeBody16 SomeBody16 requested a review from brycetham February 8, 2024 10:38
@SomeBody16 SomeBody16 changed the title Fnowakow/sdpchange event feat: sdpchange event Feb 8, 2024
src/peer-connection.ts Outdated Show resolved Hide resolved
src/peer-connection.ts Outdated Show resolved Hide resolved
src/peer-connection.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@SomeBody16 SomeBody16 requested a review from brycetham February 8, 2024 16:16
@bbaldino
Copy link
Collaborator

bbaldino commented Feb 8, 2024

I think we should consider here what the native events look like and think about being consistent: the real events don't fire an event with the SDP on createOffer, for example, but instead createOfferOnSuccess, I believe. Same for setLocalDescription, etc.

Copy link
Contributor

@brycetham brycetham left a comment

Choose a reason for hiding this comment

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

Looks good! Just one small comment on the unit tests.

Also, for completeness, maybe we should add an event for createAnswerOnSuccess too. We don't use that API much but we might as well add it while we're working on these other events.

Let's also update the PR description to reflect your recent changes, in case we want to refer back to this PR in the future.

src/peer-connection.spec.ts Outdated Show resolved Hide resolved
@SomeBody16 SomeBody16 requested a review from brycetham February 13, 2024 15:53
@SomeBody16 SomeBody16 changed the title feat: sdpchange event feat: set/create events Feb 14, 2024
@SomeBody16 SomeBody16 merged commit 92e01d9 into main Feb 14, 2024
1 check passed
@SomeBody16 SomeBody16 deleted the fnowakow/sdpchange_event branch February 14, 2024 15:47
bbaldino pushed a commit that referenced this pull request Feb 14, 2024
# [2.5.0](v2.4.0...v2.5.0) (2024-02-14)

### Features

* set/create events ([#71](#71)) ([92e01d9](92e01d9))
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.

4 participants