Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@PiN73
Copy link
Contributor

@PiN73 PiN73 commented Mar 11, 2021

This PR is a subset of #3671 and includes changes only in video_player_platform_interface package, as required in CONTRIBUTING.md

@google-cla google-cla bot added the cla: yes label Mar 11, 2021
@PiN73 PiN73 mentioned this pull request Mar 11, 2021
11 tasks
@stuartmorgan-g stuartmorgan-g self-requested a review March 17, 2021 20:26
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Just one small change, otherwise looks good.


/// HTTP headers used for the request to the [uri].
/// Only for [DataSourceType.network] videos.
Map<String, String>? httpHeaders;
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-nullable with empty default; see comment in other review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM!

@aliyazdi75
Copy link

This seems to have no httpHeaders in

@PiN73
Copy link
Contributor Author

PiN73 commented Mar 25, 2021

@aliyazdi75 yes, because this PR is just platform_interface part of #3671 which isn't merged yet

@aliyazdi75
Copy link

@PiN73 oh, ok. They must be separated, for a PR?

@PiN73
Copy link
Contributor Author

PiN73 commented Mar 25, 2021

@aliyazdi75 yes, as described in here.

In short, the reason is that video_player depends on video_player_platform_interface, new version of which must be published to pub.dev first

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants