With what? pcOfferer and pcAnswerer ?

Well this patch uses PC1 and PC2. Offerer/Answerer is clear about the roles. But it is not much better then Local/Remote in terms of being able to have more then two PC's.

FWIW, we use something1 and something2 in a bunch of the C++ unit tests and it can make it pretty hard to figure out who's who.

Comment on attachment 8557324 [details] [diff] [review] bug_1128046_rename_pcLocal.patch Review of attachment 8557324 [details] [diff] [review]: ----------------------------------------------------------------- \o/ FWIW I like pc1 and pc2, as it seems a given that one calls two. We could also go with the industry standard Alice and Bob... I used that in my fiddles http://jsfiddle.net/zy3e8rqo ::: dom/media/tests/mochitest/test_peerConnection_syncSetDescription.html @@ -41,5 @@ > var test = new PeerConnectionTest(); > test.setMediaConstraints([{video: true}], [{video: true}]); > - test.chain.replace(test, "PC_LOCAL_SET_LOCAL_DESCRIPTION", PC_LOCAL_SET_LOCAL_DESCRIPTION_SYNC); > - test.chain.replace(test, "PC_REMOTE_SET_REMOTE_DESCRIPTION", PC_REMOTE_SET_REMOTE_DESCRIPTION_SYNC); > - test.chain.replace(test, "PC_REMOTE_SET_LOCAL_DESCRIPTION", PC_REMOTE_SET_LOCAL_DESCRIPTION_SYNC); Here's a good example of where the old names were confounding. The relative terms "local" and "remote" have meaning from the point of view of one party. Additionally imposing a point-of-view narrative of the two parties is what's confusing, and shouldn't be necessary.

Fair enough maybe 1 and 2 is equally confusing. I quite like jib's proposal of Alice and Bob, because we could add Charlie later if needed.

Comment on attachment 8557324 [details] [diff] [review] bug_1128046_rename_pcLocal.patch Review of attachment 8557324 [details] [diff] [review]: ----------------------------------------------------------------- Not much to say really. 1 and 2 works fine. I suggested offerer and answerer since those are their usual roles. I wouldn't use these default variables in a test that required more than two peers, if one ever existed. Keep in mind that PeerConnection is inherently a two-party system. r(meh) from me.

Byron just wrote some code in 1017888 which swaps pcLocal and pcRemote (easiest "solution" for switching offerer and answerer in our tests right now). I think this points out that "pcOfferer" and "pcAnswerer" and also "pc1" and "pc2" would be bad names, because after swapping the two PeerConnectionWrappers the logging in going to screwed up equally as it would/will be now. So either we call everything (function names in templates.js & the labels in PeerConnectionWrapper "Alice" and "Bob"), or we rename the functions in template.js to Offerer and Answerer, but use Alice and Bob as the labels for the PeerConnectionWrappers. The latter would allow us to still have reasonable logging after swapping around the references to the different PeerConnectionWrappers.

Mass change P4->P5 to align with new Mozilla triage process.

I don't have the capacity any more to work on this any time soon.