kurento-utils-js icon indicating copy to clipboard operation
kurento-utils-js copied to clipboard

General refactor and update of used APIs

Open piranna opened this issue 5 years ago • 10 comments

Change-Id: I934e1df0623cad07faf0c2d1e38ca3dc011cc563

What is the new behavior provided by this change?

This pull-request has a full refactor of kurento-utils project using latests WebRTC Promise-based APIs and a new architecture based on ES6 classes, while at the same do some tricks to maintain backwards compatibility (they are clearly identified in case we want to remove them). In addition to that, it automatically makes use of updated adapter.js library so there's no need to do have hacks and polyfills in the code, and adds support to be used in Node.js (only non deprecated functionality, nor functionality that depends on DOM). Unit tests have been updated too to run in Node.js using Jest instead of using qunit in a browser with some hacks to extract the results.

The only thing that's backwards incompatible has been the removal of hark library since it was not being used anywhere and was only being re-exported, so it's better to require it directly if needed, but can be re-added again.

How has this been tested?

New Jest based unit tests have been added and increased code coverage. In addition to that, it's being used in production by Veedeo.me, that has been sponsoring me this refactor and its needed pull-requests for other projects like node-webrtc.

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [x] 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)
  • [x] My change requires a change to the documentation
  • [ ] My change requires a change in other repository

Checklist

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

piranna avatar Aug 05 '20 19:08 piranna

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.

jenkinskurento avatar Aug 05 '20 19:08 jenkinskurento

Thanks @piranna for the nice work. I can approve all features are working as expected (check veedeo.me) including screen sharing. We hope this helps the community to keep the Kurento project growing.

mxk1011 avatar Aug 05 '20 20:08 mxk1011

This is amazing

Thank you very much @piranna and the folks at Veedeo.me for sponsoring such an (already overdue) refactor to kurento-utils!

It is always a pleasure for all of us seeing when commercial companies are able to build solutions and extract value from open source efforts, and later decide to work on and improve those same open source projects to contribute back and help the community. This is the way it should be, and it reinforces the kind of synergies and common good that open source philosophy aims to achieve.

We happen to be focusing on other pieces of the puzzle in Kurento, and Summer vacations are also happening, so probably will need some weeks until we are able to review this. But rest assured the contribution is well appreciated :-)

j1elo avatar Aug 06 '20 10:08 j1elo

Thanks to you, this kind of comments are the ones that keep me to continue contributing on Open Source projects :-)

piranna avatar Aug 06 '20 10:08 piranna

When you are going to release?

matinzd avatar Aug 19 '20 11:08 matinzd

Any movement on this @micaelgallego?

Tenkir avatar Dec 28 '20 19:12 Tenkir

Any updates on this PR?

SamuelWei avatar Apr 16 '21 16:04 SamuelWei

The PR looks very good.

Try to use it yourself in your application code.

We don't have time right now to review it and accept it in Kurento codebase, but is the community give us feedback about it, it will be easier for us to accept it in the future.

micaelgallego avatar Apr 16 '21 16:04 micaelgallego

The PR looks very good.

Try to use it yourself in your application code.

We don't have time right now to review it and accept it in Kurento codebase, but is the community give us feedback about it, it will be easier for us to accept it in the future.

Thank you for your quick response. I'll try to get it working. As it is quite different and with no public api docs, it might take some time.

SamuelWei avatar Apr 16 '21 16:04 SamuelWei

As it is quite different and with no public api docs, it might take some time.

As already said in the PR message, it's fully backwards compatible except for the removal of the re-exported hark library, that's better to include it yourself if you need it. In addition to that, all async methods that accept a callback are now returning a Promise object too.

In the other hand, besides the full code and architecture change, tests are passing and it's being used by veedeo.me (now callab.me).

piranna avatar Apr 16 '21 17:04 piranna