Closed ivanlisovyi closed 2 years ago
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!
and we'll verify it.
ℹ️ Googlers: Go here for more info.
@googlebot I signed it!
Please, add the SPM support
For me it is failing:
@ramunasjurgilas you gotta specify the fork's URL @ master
branch, not a version tag.
Could someone please get this merged? 🙏
@ivanlisovyi there's a merge conflict FYI.
On Sun, Jan 24, 2021 at 7:18 PM Ramūnas notifications@github.com wrote:
For me it is failing: [image: image] https://user-images.githubusercontent.com/255732/105650281-fd824380-5ebb-11eb-8f17-f8a865dada31.png
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/youtube/youtube-ios-player-helper/pull/411#issuecomment-766481185, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQR7RC24DWU2ASYJMF3CY7TS3TBHXANCNFSM4RSKIVSA .
-- Erica K Vance
+1 on this!
+1. Get this merged!
👍🏼
👍 Get this merged!
@todd-patterson Can you get this merged or tag who can? Very important for modern iOS development.
Or perhaps @rcvrgs? Can you get this merged or tag who can? Very important for modern iOS development.
@todd-patterson @rcvrgs Do you guys hear that? It's a crowd chanting!
"Let's Go YouTube Developers; Let's Go!" thump thump thump thump thump
✺◟( ͡° ͜ʖ ͡°)◞✺ ✺◟( ͡ ͜ ʖ ͡ )◞✺ ✺◟(◕‿◕✿)◞✺ ✺◟( ͡ ͜ ʖ ͡ )◞✺ ✺◟( ͡° ͜ʖ ͡°)◞✺ ✺◟( ͡ ͜ ʖ ͡ )◞✺ ✺◟(◕‿◕✿)◞✺ ✺◟( ͡ ͜ ʖ ͡ )◞✺ ✺◟( ͡° ͜ʖ ͡°)◞✺
We got this!!! Let's bring it home! Let's merge this PR :D
Can we get this merged? Would be really helpful! Thanks!
I've filed a ticket with Google asking for them to ensure this package has a maintainer to look at issues like this! Hopefully that helps! https://issuetracker.google.com/issues/192913796
(Please don't brigade the ticket, just put link for cross reference.)
LGTM!
Hoping this gets merged 🤞
@googlebot or @google-admin Could you please merge this pull request as everything seems good and ready for prod (works well on fork, no conflict ... ). Thanks a lot in advance! 🙏🏻❤️
Seems like this thing isn't gonna get merged unless somebody knows somebody at YouTube engineering.
To all who are still waiting for this PR to be merged 😅
I’ve have created my own Swift Package named YouTubePlayerKit with SPM support.
https://github.com/SvenTiigi/YouTubePlayerKit
YouTubePlayerKit works basically the same way as youtube-ios-player-helper does but comes with extended support for SwiftUI, Combine, async/await and runs on iOS & macOS.
Maybe some people find this helpful ✌️
@SvenTiigi Dang; seems really promising! I think the big question is whether Apple will accept apps with this framework. It's one thing to mention in an App Store review "Hey this is officially made by YouTube itself; they'd never develop something that break its own TOS" and another to take it at your word that YouTube TOS are not broken :) Of course, I trust you but will those in the App Store Review panel?
Edit: WOW you made the "What's New" framework! You definitely know your stuff :D I really hope Apple will not complain about integrating it; I will be the first to use it if that's the case 💯
Hi @acosmicflamingo as YouTubePlayerKit has just been released I currently have no proof that an App including this framework will definitely pass the App Store Review.
But in my opinion I think this shouldn't cause a problem because at the end YouTubePlayerKit simply displays a WKWebView
which loads the YouTube player in an iFrame
by making use of the official YouTube iFrame Player API exactly like youtube-ios-player-helper does.
@SvenTiigi That is so cool. Great work!!!
whether Apple will accept apps with this framework
@acosmicflamingo This shouldn't be a problem, we did the same thing internally and it has been published to AppStore already.
@SvenTiigi Great job!
I verified the PR in the forked repo using:
Thank you for the PR @ivanlisovyi. LGTM!
Thanks @ivanlisovyi ! Very comprehensive. Discussed and reviewed with @rcvrgs
Sorry everyone for the long delay on the merge.
@todd-patterson Too late, I switched to the great "YouTubePlayerKit" made by @SvenTiigi, far better than the official YouTube-helper, videos loads faster, API is better and far more modern.
Context
This PR adds support for Swift Package Manager Installation using SPM 5.3. Closes #396.
Notable Changs
Classes
directory toSources
. This aligns the directory name with SPM default directory name requirement and since the directory now containsAssets
folder, I thinkSources
name is a better match.OCMock
to 3.7.1 to make the block with arguments invocations easier.Assets
folder fromyoutube-ios-player-helper
toSource
to match SPM requirements.youtube-ios-player-helper
to match the changes above.README.md
with the paragraph about the Swift Package Manager installation.Tested