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.

Sign up for GitHub

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

Jump to bottom

Sample: Master Cleanup #313

Merged
merged 7 commits into from
Apr 18, 2024
Merged

Sample: Master Cleanup #313

merged 7 commits into from
Apr 18, 2024

Conversation

sirknightj
Copy link
Contributor

Description of changes

  • Move common code that can be shared between master and viewer to a separate class w/ appropriate public interfaces. This makes the AWS SDK calls abstracted out and make the sample more readable and easy to modify.

Testing

Manual testing:

  • Local changes (master, chrome) --> Deployed JS Webpage (viewer, chrome)
  • Local changes (master, firefox) --> Deployed JS Webpage (viewer, chrome)
  • Local changes (master, safari) --> Deployed JS Webpage (viewer, chrome)
  • Local changes (master, chrome) --> Deployed JS Webpage (viewer, chrome) sending/receiving datachannel messages on both sides
  • Local changes (master, chrome) --> iOS (viewer)
  • Local changes (master, chrome) --> Android (viewer)
  • Local changes (master, chrome) --> WebRTC ingestion [success case]
  • Local changes (master, chrome) --> WebRTC ingestion [failure case, channel not configured for ingestion, verify that application exits]
  • Local changes (master, chrome) --> WebRTC ingestion [failure case, verify the 5 connections within 10 minutes]
  • Local changes (master, firefox) --> WebRTC ingestion [failure case, verify that statusResponse is still printed and application exits]

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sirknightj sirknightj added Samples Questions related to Samples cleanup labels Apr 16, 2024
disa6302
disa6302 reviewed Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@disa6302 disa6302 left a comment

Choose a reason for hiding this comment

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

Awesome work!

examples/channelHelper.js Show resolved Hide resolved
examples/channelHelper.js Show resolved Hide resolved
stefankiesz
stefankiesz approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@stefankiesz stefankiesz left a comment

Choose a reason for hiding this comment

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

Looks great to me!

niyatim23
niyatim23 approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@niyatim23 niyatim23 left a comment

Choose a reason for hiding this comment

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

Looks awesome! One question though, when we modify this for the viewer (which has the profiling logic embedded in the code we moved to channelHelper), do we intend to support profiling from Master and Viewer both?

@sirknightj
Copy link
Contributor Author

We can modify this afterwards to add that functionality when needed.

@sirknightj sirknightj merged commit fa49653 into develop Apr 18, 2024
4 checks passed
@sirknightj sirknightj deleted the cleanup branch April 18, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@disa6302 disa6302 disa6302 approved these changes

@niyatim23 niyatim23 niyatim23 approved these changes

@stefankiesz stefankiesz stefankiesz approved these changes

Assignees
No one assigned
Labels
cleanup Samples Questions related to Samples
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sirknightj @disa6302 @niyatim23 @stefankiesz