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.

Sign up for GitHub

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

Jump to bottom

fix(MCap): Remove robustness when robustness value is default #4953

Merged
merged 1 commit into from
Jan 31, 2023
Merged

fix(MCap): Remove robustness when robustness value is default #4953

merged 1 commit into from
Jan 31, 2023

Conversation

avelad
Copy link
Member

@avelad avelad commented Jan 31, 2023

Fixes #4659

@avelad avelad added type: bug Something isn't working correctly platform: TV/STB Issues affecting smart TV or set-top box platforms priority: P2 Smaller impact or easy workaround labels Jan 31, 2023
@avelad avelad added this to the v4.4 milestone Jan 31, 2023
@github-actions
Copy link
Contributor

Incremental code coverage: 100.00%

joeyparrish
joeyparrish approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

A regression test would be really good to have for this. This is exactly the sort of thing someone could easily "clean up" to be helpful.

joeyparrish
joeyparrish requested changes Jan 31, 2023
View reviewed changes
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Looks like this does not work on Firefox or Safari for some reason.

@avelad
Copy link
Member Author

avelad commented Jan 31, 2023 •
edited
Loading

@joeyparrish The tests are failing, I'm investigating why. There are a problem with MCap polyfill

@avelad avelad force-pushed the mediacapabilities-robustness branch from cdd6c81 to 2e9e2a7 Compare January 31, 2023 16:15
@avelad avelad force-pushed the mediacapabilities-robustness branch from 2e9e2a7 to 2734699 Compare January 31, 2023 16:41
joeyparrish
joeyparrish approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

LGTM. Are you planning to add a regression test still? If so, I'll come back to review it again then.

@avelad
Copy link
Member Author

avelad commented Jan 31, 2023

@joeyparrish At the moment, I believe that the current tests cover the necessary cases.

@avelad avelad merged commit 7439a26 into shaka-project:main Jan 31, 2023
@avelad avelad deleted the mediacapabilities-robustness branch January 31, 2023 17:22
@github-actions github-actions bot mentioned this pull request Jan 31, 2023
Merged
joeyparrish pushed a commit that referenced this pull request Feb 9, 2023
@avelad @joeyparrish
fix(MCap): Remove robustness when robustness value is default (#4953)
6115dd7 
Fixes #4659
joeyparrish pushed a commit that referenced this pull request Feb 9, 2023
@avelad @joeyparrish
fix(MCap): Remove robustness when robustness value is default (#4953)
762b1e7 
Fixes #4659
joeyparrish pushed a commit that referenced this pull request Feb 9, 2023
@avelad @joeyparrish
fix(MCap): Remove robustness when robustness value is default (#4953)
59bbb56 
Fixes #4659
joeyparrish pushed a commit that referenced this pull request Feb 9, 2023
@avelad @joeyparrish
fix(MCap): Remove robustness when robustness value is default (#4953)
61c8a06 
Fixes #4659
This was referenced Feb 9, 2023
Merged
Merged
Merged
Merged
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joeyparrish joeyparrish joeyparrish approved these changes

Assignees
No one assigned
Labels
platform: TV/STB Issues affecting smart TV or set-top box platforms priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v4.4
Development

Successfully merging this pull request may close these issues.

Allow to remove the robustness from the equation instead of defaulting to an empty string
2 participants
@avelad @joeyparrish