Skip to content

add vlc options from flutter#64

Closed
pharshdev wants to merge 3 commits into
solid-software:masterfrom
pharshdev:master
Closed

add vlc options from flutter#64
pharshdev wants to merge 3 commits into
solid-software:masterfrom
pharshdev:master

Conversation

@pharshdev

Copy link
Copy Markdown

i think it would be nice to have this.

@solid-software

Copy link
Copy Markdown
Collaborator

Nice 👍 thanks for your contribution @pharshdev!

Is there a way you can remove "System.out.println" - so we can have more clean console for users + update changeling.md to include this new version changes?
Also should we add same options for iOS part?

@mitchross if you have time - could you please take a look and let us know what you think about it?

@mitchross

Copy link
Copy Markdown
Contributor

I will review.

@alr2413

alr2413 commented Jun 30, 2020

Copy link
Copy Markdown
Contributor

ping @solid-software
When will you merge it?

@mitchross mitchross left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ping @solid-software
When will you merge it?

When comments are resolved.

}

Future<void> _initialize(String url) async {
Future<void> _initialize(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you should either make a new function called initializeWithOptions or use optional parameters. https://stackoverflow.com/questions/49734608/function-overloading-in-dart

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

API versioning states that no breaking changing. This is technically a breaking change. So lets fix that by providing optional params or a new initialization method. The latter is preferred.

// with LibVLC on the platform side.
if (_controller.hasClients) {
await _controller._initialize(widget.url);
await _controller._initialize(widget.hwAcc, widget.options, widget.url);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see comment below

@alr2413

alr2413 commented Jul 28, 2020

Copy link
Copy Markdown
Contributor

ping @pharshdev.

Please resolve the mentioned conflicts.
Thanks.

@mitchross

Copy link
Copy Markdown
Contributor

If @pharshdev doesnt take care of this, ill fork and take care of it this week.

@mitchross

Copy link
Copy Markdown
Contributor

Fix for this PR #84

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants