Skip to content

WebRTC Additions #203

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
Merged
merged 1 commit into from
Jan 26, 2016
Merged

WebRTC Additions #203

merged 1 commit into from
Jan 26, 2016

Conversation

@coreyauger
Copy link
Contributor

No description provided.

@mseddon
Copy link
Contributor

mseddon commented Jan 21, 2016

Ah, yes, MediaStream was moved, hence build errors:

https://github.com/coreyauger/scala-js-dom/blob/master/src/main/scala/org/scalajs/dom/package.scala#L106 remove this reference to MediaStream

https://github.com/coreyauger/scala-js-dom/blob/master/src/main/scala/org/scalajs/dom/raw/Audio.scala#L12 import org.scalajs.dom.mediastream.MediaStream here.
mseddon
mseddon reviewed Jan 21, 2016
View reviewed changes
src/main/scala/org/scalajs/dom/experimental/mediastream/MediaStream.scala Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Three URLS? :)
@coreyauger
Copy link
Contributor Author

fixed.
mseddon
mseddon reviewed Jan 21, 2016
View reviewed changes
src/main/scala/org/scalajs/dom/experimental/mediastream/MediaStream.scala Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

trackId: String - best to use the name in the spec and fix the spacing around the :
@mseddon
Copy link
Contributor

mseddon commented Jan 21, 2016

That's all.
sjrd
sjrd reviewed Jan 22, 2016
View reviewed changes
src/main/scala/org/scalajs/dom/experimental/mediastream/MediaStream.scala Outdated
Copy link
Member

Choose a reason for hiding this comment

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

No need for "READONLY". We know it's read-only thanks to our type system (it's a val or def)
@sjrd
Copy link
Member

sjrd commented Jan 22, 2016

That's all.
@kapunga kapunga mentioned this pull request Jan 26, 2016
Closed
@sjrd
Copy link
Member

sjrd commented Jan 26, 2016

OK could you please squash the two commits into one?
@coreyauger
Copy link
Contributor Author

Done. Thanks :)
@sjrd
Copy link
Member

sjrd commented Jan 26, 2016

LGTM
sjrd added a commit that referenced this pull request Jan 26, 2016
@sjrd
Merge pull request #203 from coreyauger/webrtc-api
f7117a0 
WebRTC Additions
@sjrd sjrd merged commit f7117a0 into scala-js:master Jan 26, 2016
@mseddon
Copy link
Contributor

mseddon commented Jan 26, 2016

👍 thanks for all your effort @coreyauger, much appreciated!
@danielwegener danielwegener mentioned this pull request Feb 6, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants

@coreyauger @mseddon @sjrd