Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Use IceCandidateFound event instead of deprecated OnIceCandidate event #9

Merged
merged 1 commit into from Aug 6, 2020
Merged

Use IceCandidateFound event instead of deprecated OnIceCandidate event #9

merged 1 commit into from Aug 6, 2020

Conversation

whitphx
Copy link
Contributor

@whitphx whitphx commented Aug 6, 2020

What is the current behavior you want to change?

Current code is using a event OnIceCandidate though the doc says it is deprecated:
https://doc-kurento.readthedocs.io/en/stable/_static/client-jsdoc/module-elements.html#event:OnIceCandidate

What is the new behavior provided by this change?

Use IceCandidateFound event instead as the doc says.

How has this been tested?

I ran the tutorials.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature / enhancement (non-breaking change which improves the project)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • My change requires a change to the documentation
  • My change requires a change in other repository

Checklist

  • I have read the Contribution Guidelines
  • I have added an explanation of what the changes do and why they should be included
  • I have written new tests for the changes, as applicable, and have successfully run them locally

@jenkinskurento
Copy link

Hi there, thanks for your Pull Request!

A Kurento member needs to verify that this patch is reasonable to test. In case it is, they should write a comment with the phrase test this please. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by Kurento members will still work. Regular contributors can be whitelisted to skip this step.

@j1elo
Copy link
Member

j1elo commented Aug 6, 2020

Browser JS tutorials do not mention usage of OnIceCandidate event, so the tutorial text doesn't need update (contrary to what happens with Node tutorials).

There is a bunch of renames planned for Kurento 7, but for now this is a good first step in updating all client code. Thanks you!

@j1elo j1elo merged commit b6510bd into Kurento:master Aug 6, 2020
@whitphx
Copy link
Contributor Author

whitphx commented Aug 6, 2020

Thank you for acceptance :)

There is a bunch of renames planned for Kurento 7

I see, and I'm sorry if it was ahead of the original plan.
I just got confused when reading both the reference and the tutorial, and should have asked in the mailing list first.

@whitphx whitphx deleted the fix/deprecated-on-ice-candidate-event branch August 6, 2020 12:32
@j1elo
Copy link
Member

j1elo commented Aug 6, 2020

I just got confused when reading both the reference and the tutorial, and should have asked in the mailing list first.

Well this PR is in line with the renaming that was on the line, but yes, it's better to first ask because otherwise you might do some work that in the end is not accepted because it conflicts with some other development or planning. But otherwise thanks for your interest in Kurento and your contributions! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants