-
Notifications
You must be signed in to change notification settings - Fork 39
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
@nstudio/nativescript-exoplayer renaming and documentation updates #68
Comments
I'm still very interested in finding a good way to inform the community through the docs about the different video players(as well as improve them so new community members can get started with video quickly). When I started my {N} journey, I was very confused as to which player to use. I tested out random marketplace players and several other that were in the community. The goal: What I understand now: I get the reason for having both plugins, so that devs that just need a slimmed down video player use @nstudio/nativescript-videoplayer. Then the devs that need more features and are fine with the added packages that come with exoplayer, use @nstudio/nativescript-exoplayer. A possible solution: Create a @nativescript/basic-videoplayer (Apple AVPlayer and Android MediaPlayer) and a @nativescript/advanced-videoplayer (Apple AVPlayer and Google ExoPlayer). Add links in the plugin section of the {N} docs like other plugins have. Then in each repo, add somewhat concise wording to the readme describing what the main features/benefits of each. This could also be partially documented in a blog post or in the {N} docs. I believe doing something like this would create more community awareness, allow people trying out NativeScript to see how easy it could be to get video going quickly and likely lead to better plugins overall. If this type of idea is unwanted or there are reasons why it shouldn't happen I'd be interested to hear those as well. I'm interested in finding a good solution to move forward with and looking for other's opinions too! I'm not sure who to chat with so I'm tagging @nstudio @NathanWalker |
Great thoughts here @brianrclow thanks for outlining this. I think the naming could be: The advanced naming probably makes sense because the API would also be more advanced and thorough which would likely prove the value of having both since not all cases would need the more advanced player. |
Thanks @NathanWalker Yea, I think naming like that would make more sense. What would be the first steps in creating a timeline to accomplish something like this? I would be interested in helping out with documentation and figuring out how to add a few more features to the plugins. |
Big help! You could fork https://github.com/NativeScript/plugins
npm run add
and begin moving the plugins in there - once readmes are updated in there we have nightly scripts that would add them to the docs |
Okay great, thanks for the direction. I'll get this started in an upcoming sprint, need to balance paid work and OSS. |
@NathanWalker I've forked, created the new packages under the @NativeScript scope and started adding in the readme's. Creating a new plugin in that monorepo is pretty slick and clean. Before I get too far, I wanted to ask you some questions:
Here is where I am currently working on it: https://github.com/brianrclow/plugins/tree/video-advanced I'm going to continue to add to the readme's and try to understand some of the code better so I can make a smoother transition. Any advice is appreciated, thanks! |
Also - I've noticed that all of the commits are using the conventional commit pattern, is there a place where Nativescript has the guidelines around that for community members to understand how to best commit their code? I don't want to do it wrong and mess up things. 😅😅 |
Your fork looks like great direction - yes, you can copy/paste into that new package, |
Best way is for us to mention in the README here for the exoplayer the proper credit and keep the commits here for history. That way it's clear where the credit is due and how to find it. The new package can also link and mention the original for proper history investigations. |
The plugin workspace fork you have is only for {N} 7 and above; all plugin workspaces support 7 and above only. |
Great question, we try to follow this format: https://www.conventionalcommits.org/en/v1.0.0/ |
@NathanWalker @brianrclow about the naming / split of the 2 plugins . wouldn't it be better to make exoplayer an "extension" to the other plugin? |
I'll leave my thoughts here as well: Most of the time, simply crediting the prior art/work in the repo/readme is enough (though do always check the license, and if necessary include a copy of it). Preserving git history is possible, I have done it before, however it requires quite a bit of git trickery, and it's not perfect. An example where I did this: https://github.com/NativeScript/nativescript-cli/tree/master/packages/doctor The upside is that all the prior commits are preserved, however merge commits now point to incorrect pull requests, since Is it worth the trouble? Maybe? Not really... About organizing and splitting, I'd agree with @farfromrefug's take that it might make sense to have a base plugin with a nice api, and make the underlying player implementation swappable with a single call in main.ts or something simple like that. |
I was considering this recently and I think the license point is important, and depending on the licence does appear to place specific requirements on the copy. for example the license governing exoplayer here is apache 2 and explicitly states ( among other things) in section 4 "(a) You must give any other recipients of the Work or Derivative Works a copy of this License". My reading of this is the contents of https://raw.githubusercontent.com/nstudio/nativescript-plugins/main/LICENSE must be brought across to the new repo, and furthermore must end up in the npm package. Also there are instructions for indicating what files are changed etc. so there are requirements placed on any copy by the license. The reason I was thinking about it was that I was considering similar to this, bringing https://github.com/EddyVerbruggen/nativescript-secure-storage into NativeScript plugins and rewriting the android portion to use EncryptedSharedPreferences. ( it is MIT license, which is simpler but also requires the original license travels with the code ). I find this not very straight forward, but it would seem to me to be important to understand ( which I probably do not fully ) |
@brianrclow looks like this escalated quickly 🤣 ... some considerations here, great input from everyone. |
Thank you all for the feedback! @NathanWalker I will get started on bringing over exoplayer to @farfromrefug I'm not against making the exoplayer an "extension" to the other plugin like you say. I haven't seen this done before though and I wouldn't know how to accomplish that. There is some value in only maintaining "1 iOS side" instead of duplicating that in both the "video" and "video-advanced" versions. How might you suggest going about the extension process? Is there another plugin that follows this I could check out? @rigor789 Preserving history sounds like it would be a bit of a pain and I'm not sure if I'm up for it. Thanks for sending those links though. I would still want to follow the licenses and give credit to prior work done. As for the plugin extension idea/implementation. Do you have any example of one currently with a similar setup? I think that idea would simplify things further, which is my goal. Having a single plugin that has the capability of changing the Android native implementation sounds like a better solution than 2 plugins that have the same iOS native implementation. @jcassidyav Good call on the license. It's something I wanted to be careful about. The I'm very interested in this project, both because of an app I have in production that utilizes video and for the greater community moving forward. I think making video clear to use and adding important features will be a good improvement to the developer experience with NativeScript, especially for newer developers coming from a web background. I'll definitely need to lean on some community members/maintainers for this as time goes on as I've never written or assisted with a plugin like this before! |
Over the past few months I have noticed some confusion around using video with nativescript. The main issue stems from exoplayer and videoplayer both being used but not clear which should be used or why. The idea would be to potential renaming @nstudio/nativescript-exoplayer using the word "video" but being clear that it is more robust and requires the exo libraries with it.
It would be nice to also include some sort of video section in the UI docs or including it in the plugins section of the official nativescript website. This would ideally solve some confusion around if nativescript supports video and which plugin would be best for your project.
I believe it would be helpful if it is more clear that nativescript can support video and does, as well as give some information around which plugin might be better for the project someone is currently working on. This might also create more interest in PRs for the plugins as they will be more visible.
The text was updated successfully, but these errors were encountered: