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

feat: Remove streaming.parsePrftBox config #7358

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

avelad
Copy link
Member

@avelad avelad commented Sep 24, 2024

The site code is changed to use a parser that is always used and thus we avoid having a configuration that is not necessary.

@avelad avelad requested review from theodab and tykus160 September 24, 2024 07:55
@avelad avelad added type: enhancement New feature or request priority: P3 Useful but not urgent labels Sep 24, 2024
@avelad avelad added this to the v4.12 milestone Sep 24, 2024
@shaka-bot
Copy link
Collaborator

shaka-bot commented Sep 24, 2024

Incremental code coverage: 56.90%

@avelad avelad force-pushed the remove-parsePrftBox branch from feab35c to 6c6d3ca Compare September 24, 2024 08:37
Copy link
Contributor

@theodab theodab left a comment

Choose a reason for hiding this comment

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

Would you say we would need to have a deprecation warning for this removed config value in shaka.Player.configure? Or was it too recent for that, I'm not sure.

@avelad
Copy link
Member Author

avelad commented Sep 24, 2024

Would you say we would need to have a deprecation warning for this removed config value in shaka.Player.configure? Or was it too recent for that, I'm not sure.

Since before the event was only fired when the configuration was true (default false). And now we always fire the event. It's not a problem

@avelad avelad requested a review from theodab September 24, 2024 12:01
@theodab
Copy link
Contributor

theodab commented Sep 24, 2024

Would you say we would need to have a deprecation warning for this removed config value in shaka.Player.configure? Or was it too recent for that, I'm not sure.

Since before the event was only fired when the configuration was true (default false). And now we always fire the event. It's not a problem

I was thinking more so that we don't create an error log for people who are currently setting the configuration value to true. It can say that the event is now fired without needing a configuration value, perhaps.
Maybe that's not really necessary though.

@avelad
Copy link
Member Author

avelad commented Sep 24, 2024

Would you say we would need to have a deprecation warning for this removed config value in shaka.Player.configure? Or was it too recent for that, I'm not sure.

Since before the event was only fired when the configuration was true (default false). And now we always fire the event. It's not a problem

I was thinking more so that we don't create an error log for people who are currently setting the configuration value to true. It can say that the event is now fired without needing a configuration value, perhaps. Maybe that's not really necessary though.

Done!

@avelad avelad merged commit fc4893d into shaka-project:main Sep 24, 2024
14 of 15 checks passed
@avelad avelad deleted the remove-parsePrftBox branch September 25, 2024 07:31
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Nov 23, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Nov 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants