-
Notifications
You must be signed in to change notification settings - Fork 681
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 Swift Package Manager Support #411
Conversation
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). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Please, add the SPM support |
@ramunasjurgilas you gotta specify the fork's URL @ |
Could someone please get this merged? 🙏 @ivanlisovyi there's a merge conflict FYI. |
- Add support for Swift Package Manager (SPM 5.3 required) - Fix failing unit tests - Remove redudant unit tests due to public API changes - Fix warnings for test target
On Sun, Jan 24, 2021 at 7:18 PM Ramūnas ***@***.***> 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
<#411 (comment)>,
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!" ✺◟( ͡° ͜ʖ ͡°)◞✺ ✺◟( ͡ ͜ ʖ ͡ )◞✺ ✺◟(◕‿◕✿)◞✺ 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.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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 ... ). |
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 |
@SvenTiigi That is so cool. Great work!!! |
@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. 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