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

setConfiguration({ peerIdentity }) when there is no initial peerIdentity #1464

Closed
wants to merge 3 commits into from

Conversation

aboba
Copy link
Contributor

@aboba aboba commented Jul 7, 2017

Fix for Issue #1456


Preview | Diff

@aboba aboba requested a review from taylor-b July 7, 2017 05:08
@soareschen
Copy link
Contributor

This removes the original condition. Does this means peerIdentity can be changed if it is initially set?

@aboba
Copy link
Contributor Author

aboba commented Jul 7, 2017

@soareschen Restored the original condition. peerIdentity should not be changeable.

@martinthomson
Copy link
Member

This would be a fine answer to the question. Note that you could take the approach I stated earlier, but that's more complicated.

webrtc.html Outdated
@@ -1679,9 +1679,6 @@
set and <a>[[\Configuration]]</a>.<code>peerIdentity</code> is
not set, <a>throw</a> an <code>InvalidModificationError</code>.
</li>
<li>If <code><var>configuration</var>.peerIdentity</code> is
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the original condition removed again?

set and its value differs from the <a>target peer
identity</a>, <a>throw</a> an <code>InvalidModificationError</code>.
set and <a>[[\Configuration]]</a>.<code>peerIdentity</code> is
not set, <a>throw</a> an <code>InvalidModificationError</code>.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds like it allows the peer identity to change, just not go from being unset to being set. I don't think that's what we want. I thought the plan was to give everything a default value (null for peerIdentity, [] for certificates, etc.), and just say "if the value is different, throw an InvalidModificationError".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's better.

@aboba aboba closed this Jul 25, 2017
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.

None yet

4 participants