search

jackBonadies / SeekerAndroid

Android client for the Soulseek peer-to-peer network
GNU General Public License v3.0
263 stars 4 forks source link

Use git submodules for libraries #2

Closed flexagoon closed 2 years ago

flexagoon commented 3 years ago

Including other git repositories in your repo directly is a bad practice. Instead, they should be added with git submodules. This pull request fixes the issue.

@jackBonadies

jackBonadies commented 3 years ago

Hello :) Thanks for the help. However, I think I need further help / advice regarding this. The reason I did not include the libraries directly (either through git or nuget packages, etc) is because I have modified the code in them. For Soulseek.NET it was adding event handlers to better integrate it into seeker, a bug fix for it to work with ipv6 (that I realize I should tell the original author about), and a couple other tweaks. For the Upnp library - this library does not work with Android out of the box, it tries to read files it does not have permissions for (but otherwise would on a linux system), instead one must get ip addresses and gateways the Android way. What is the best way to go about rectifying this?

flexagoon commented 3 years ago

In that case, you should create your own forks of Soulseek.NET and Upnp, and commit your changes to those forks. With Soulseek.NET, since your changes fix an important bug, you can also create a pull request to merge your changes into the official repo.

Then you just include your forks as submodules instead of the official repositories.

I can help you with the second part in case you don't know how to use git submodules, but first you need to create the forks

flexagoon commented 3 years ago

As a bonus, that approach will also allow you to easily reuse your modified versions of those libraries in other projects.

jackBonadies commented 3 years ago

Okay thanks very much for the advice. I am just wrapping up a couple bugs but will then get to work making forks, cleaning up, and notifying author of the bug fix.

flexagoon commented 2 years ago

Hello @jackBonadies, what's the state of this?

jackBonadies commented 2 years ago

Hi, I will try to do this soon. The UPnP submodule shouldnt be too bad, just need to do a diff with the original to see what changed. Soulseek .NET will be a bit tougher.