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

Import playlist #10519

Closed
Closed

Conversation

mannith-narayan
Copy link

@mannith-narayan mannith-narayan commented Oct 28, 2023

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Import a playlist from a text file containing links of youtube videos.
  • Name of the file is the name of the playlist.
  • #9921

Before/After Screenshots/Screen Record

  • Before:
    Screenshot_20231029-085952

  • After:
    A menu button
    Import option
    The file manager is opened onClick, and the added playlist would appear as any other that is added by the user manually when watching.

Relies on the following changes

  • Made some modifications to BookmarkFragment
  • Added a new class BookmarkImportService

APK testing

  • It could take a couple of seconds for the playlist to appear. A toast message appears once the playlist is added.

Due diligence

@AudricV
Copy link
Member

AudricV commented Oct 30, 2023

As our README and our contribution guidelines (which you agreed) say, only bugfix PRs will be accepted. We have to close your pull request unfortunately as it introduces a new feature.

Here are some tips for your future pull requests on this repository:

  • Please base your branch properly on top of the upstream dev branch. The first 4 commits, which are not a part of the upstream dev branch, have nothing to do with your changes and so are not necessary for them.
  • Several of your commits seem to be "WIP commits". In NewPipe projects, we prefer generally cleaner commits so we can use them as they are when merging pull requests, allowing us to better understand PR changes in the future. You should have squashed some commits as they contain several unneeded changes at the end (like printing LOL to Java's standard output) and improved their wording (App Crashes isn't a good commit message at all in my opinion).

Please do not take this result as a bad experience but as a learning one :)

@AudricV AudricV closed this Oct 30, 2023
@AudricV AudricV added the template ignored The user didn't follow the template/instructions (or removed them) label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
template ignored The user didn't follow the template/instructions (or removed them)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants