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

fix(package): update videojs-contrib-quality-levels to 4.0.0 to eliminate deprecation warning #8303

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

Makio64
Copy link
Contributor

@Makio64 Makio64 commented Jun 2, 2023

Description

Fixed a warning when using quality-levels 3.x which used mergeOptions. It prepare for 9.0.0

Specific Changes proposed

update videojs-contrib-quality-levels to ^4.0.0 in package.json

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

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

Tests are failing and I wonder if the cause is that different versions of Node and npm were used than are expected for the build.

I did the upgrade locally and everything works with minimal changes to the package-lock.json. Here's the entire diff when I use Node 14 and npm 6:

diff --git a/package-lock.json b/package-lock.json
index 3b9d2c46..f43a1cc2 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -15224,13 +15224,21 @@
               }
             }
           }
+        },
+        "videojs-contrib-quality-levels": {
+          "version": "3.0.0",
+          "resolved": "https://registry.npmjs.org/videojs-contrib-quality-levels/-/videojs-contrib-quality-levels-3.0.0.tgz",
+          "integrity": "sha512-sNx38EYUx+Q+gmup1gVTv9P9/sPs28rM7gZOx1sedaHoKxEdYB+ysOGfHj6MSELBMNGMj6ZspdrpSiWguGvGxA==",
+          "requires": {
+            "global": "^4.4.0"
+          }
         }
       }
     },
     "videojs-contrib-quality-levels": {
-      "version": "3.0.0",
-      "resolved": "https://registry.npmjs.org/videojs-contrib-quality-levels/-/videojs-contrib-quality-levels-3.0.0.tgz",
-      "integrity": "sha512-sNx38EYUx+Q+gmup1gVTv9P9/sPs28rM7gZOx1sedaHoKxEdYB+ysOGfHj6MSELBMNGMj6ZspdrpSiWguGvGxA==",
+      "version": "4.0.0",
+      "resolved": "https://registry.npmjs.org/videojs-contrib-quality-levels/-/videojs-contrib-quality-levels-4.0.0.tgz",
+      "integrity": "sha512-u5rmd8BjLwANp7XwuQ0Q/me34bMe6zg9PQdHfTS7aXgiVRbNTb4djcmfG7aeSrkpZjg+XCLezFNenlJaCjBHKw==",
       "requires": {
         "global": "^4.4.0"
       }
diff --git a/package.json b/package.json
index b7ace26d..8589072b 100644
--- a/package.json
+++ b/package.json
@@ -96,7 +96,7 @@
     "mpd-parser": "^1.0.1",
     "mux.js": "^6.2.0",
     "safe-json-parse": "4.0.0",
-    "videojs-contrib-quality-levels": "3.0.0",
+    "videojs-contrib-quality-levels": "^4.0.0",
     "videojs-font": "4.1.0",
     "videojs-vtt.js": "0.15.4"
   },

Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

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

I went ahead and pushed up a fix here because we do want to get this in. The issue was that a version other than Node 14 and npm 6 was used and the package-lock.json was significantly changed.

Using the expected version of Node/npm resolved the test failures.

@misteroneill misteroneill added needs: LGTM Needs one or more additional approvals and removed needs: updates labels Jun 6, 2023
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #8303 (7ca942e) into main (581786a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #8303   +/-   ##
=======================================
  Coverage   82.56%   82.56%           
=======================================
  Files         112      112           
  Lines        7495     7495           
  Branches     1808     1808           
=======================================
  Hits         6188     6188           
  Misses       1307     1307           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@misteroneill misteroneill changed the title update videojs-contrib-quality-levels to ^4.0.0 fix(package): update videojs-contrib-quality-levels to 4.0.0 to eliminate deprecation warning Jun 6, 2023
@misteroneill misteroneill added confirmed and removed needs: LGTM Needs one or more additional approvals labels Jun 6, 2023
@misteroneill misteroneill merged commit fab8f95 into videojs:main Jun 6, 2023
@welcome
Copy link

welcome bot commented Jun 6, 2023

Congrats on merging your first pull request! 🎉🎉🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants