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

add list item to play video on kodi #5310

Merged
merged 3 commits into from
Jan 13, 2021
Merged

add list item to play video on kodi #5310

merged 3 commits into from
Jan 13, 2021

Conversation

khimaros
Copy link
Contributor

@khimaros khimaros commented Dec 30, 2020

this allows the user to long press on a video in any list (search, history, playlist, etc) and send it directly to Kodi. this change honors the existing Show "Play with Kodi" option setting and only displays on streams supported by Kore.

there is opportunity to reduce duplication in this part of the code base generally.

closes: #5157

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

  • add list context menu item to "Play on Kodi"

Fixes the following issue(s)

APK testing

app-debug.zip

Due diligence

Copy link
Collaborator

@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.

If you can think of a way to add an unit test (not instrumented) that would be ideal.
Only thing i can think of is passing in service id, context and preference manager through the method and mocking the latter 2 in tests.

@khimaros
Copy link
Contributor Author

i've resolved all inline feedback. i'm afraid i'm not able to contribute instrumentation and test harness at this time.

@khimaros khimaros requested a review from Stypox December 30, 2020 22:55
XiangRongLin
XiangRongLin previously approved these changes Dec 31, 2020
@TobiGr TobiGr added the GUI Issue is related to the graphical user interface label Dec 31, 2020
@khimaros
Copy link
Contributor Author

thank you, @XiangRongLin! i believe i accidentally put @Stypox in the critical path as well, so it looks like i'll just need one more review from them before merging. thank you all for this amazing project.

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Changes look good, thank you :-D

app/src/main/java/org/schabi/newpipe/util/KoreUtil.java Outdated Show resolved Hide resolved
@khimaros
Copy link
Contributor Author

khimaros commented Jan 2, 2021

resolved remaining feedback in latest commit. please take another look.

@khimaros
Copy link
Contributor Author

khimaros commented Jan 2, 2021

@Stypox thank you, are you able to approve? it seems merge is blocked on your approval and i'm not able to remove you.

@khimaros khimaros requested a review from Stypox January 2, 2021 19:27
@khimaros
Copy link
Contributor Author

khimaros commented Jan 4, 2021

@TobiGr -- thank you for the approval. are you able to merge this or does @Stypox need to approve first? i'm not able to merge it myself.

@khimaros
Copy link
Contributor Author

khimaros commented Jan 7, 2021

@Stypox -- friendly ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI Issue is related to the graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show "Stream to Kodi" in context menu
4 participants