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

Added: Getting started #37

Closed
wants to merge 3 commits into from
Closed

Added: Getting started #37

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 22, 2021

see

@ghost
Copy link
Author

ghost commented Dec 22, 2021

Hello @TobiGr, i did write it in java, improve it a little, and added very simple examples :)

@ghost
Copy link
Author

ghost commented Dec 24, 2021

What's Up @TobiGr :^)

@opusforlife2 opusforlife2 requested a review from TobiGr December 25, 2021 19:50
@ghost
Copy link
Author

ghost commented Jan 7, 2022

Hello @TobiGr 😄

Copy link
Contributor

@XiangRongLin XiangRongLin left a comment

Choose a reason for hiding this comment

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

Before I review more: Why are is the PR trying to merge into the branch feat/getting_started and not master?


```gradle
// NewPipe Libraries
implementation 'com.github.TeamNewPipe:NewPipeExtractor:0.21.12'
Copy link
Contributor

Choose a reason for hiding this comment

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

NewPipe versions are prefixed with a 'v', so it should be NewPipeExtractor:v0.21.12

Copy link
Author

Choose a reason for hiding this comment

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

I changed this ;)

Copy link
Author

Choose a reason for hiding this comment

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

Why are is the PR trying to merge into the branch feat/getting_started and not master?

You are right, I open new one to merge directly to master ;)

Note that you could use any http client other then okhttp, but it used here for more simplicity.


### Make sure you use Java 11
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the extractor really have the java 11 requirement? I thought that it didn't even support the full Java 8 feature set.

Copy link
Author

Choose a reason for hiding this comment

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

For some reason I wasn't able to run it below java 8 for android, I really don't know why..

However this might change when we find a solution ;)

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Comment on lines +142 to +143
<details>
<summary>simple implementation</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

idk.. both solutions seems ok

Copy link
Author

Choose a reason for hiding this comment

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

However I added a reference ;)

@ghost
Copy link
Author

ghost commented Jan 14, 2022

I close this since I did make a PR to feat not master!
I opened new PR which go directly to master ;)

please see the new PR here: #38

@ghost ghost closed this Jan 14, 2022
This pull request was closed.
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.

1 participant