-
Notifications
You must be signed in to change notification settings - Fork 425
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
refactor: clean up parameters of excludePlaylist #1304
refactor: clean up parameters of excludePlaylist #1304
Conversation
Codecov Report
@@ Coverage Diff @@
## next #1304 +/- ##
=======================================
Coverage 86.27% 86.27%
=======================================
Files 39 39
Lines 9791 9795 +4
Branches 2275 2277 +2
=======================================
+ Hits 8447 8451 +4
Misses 1344 1344
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One q around a comment. Otherwise, LGTM
src/master-playlist-controller.js
Outdated
// If the `error` was generated by the playlist loader, it will contain | ||
// the playlist we were trying to load (but failed) and that should be | ||
// excluded instead of the currently selected playlist which is likely | ||
// out-of-date in this scenario |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this comment still correct? At the very least, we won't be using the error.playlist here anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! I'll remove the whole comment.
Rename the parameters passed to excludePlaylist to make the logic easier to follow.
f383b29
to
a689041
Compare
Rebased for @gesinger to resolve conflicts due to master/main renaming. |
Rename the parameters passed to excludePlaylist to make the logic easier to follow.
Requirements Checklist