Fix dependencies - Githubissues

tlf30 / monkey-netty

A implementation of a server-client communication system for jMonkeyEngine using Netty.IO that utilizes both TCP and UDP communication.

MIT License

12 stars 5 forks source link

Fix dependencies #22

Closed demoth closed 3 years ago

demoth commented 3 years ago

Removed unnecessary dependencies (jme3 desktop & jme3 lwjgl are not requried for network communication)
jme3core dependency is now api: Since this dependency is required anyway (it is exposed to the user by NettyServer being a BaseAppState child) it should be a transitive depencency
set gradlew +x mode

tlf30 commented 3 years ago

@demoth thank you for addressing this. I am not at a computer at the moment but I see that it failed to build in the Github action. Can you please address that? I will have time this evening or tomorrow to take a look. I am currently traveling back from work over the next couple days.

demoth commented 3 years ago

yep, checking..

demoth commented 3 years ago

fixed, a wrong plugin was used

tlf30 commented 3 years ago

@demoth sorry for the delay. I finally have reached me destination, currently in Nebraska for a week.

A couple items to address here, I racked my brain trying to remember why those were added as dependencies. Finally it occurred to me that it was to run the example server/client. I agree that this library should be independent of lwjgl and desktop. The best solution would be to create a separate gradle subproject for the examples and the library. Would you be willing to tackle that in this PR?

demoth commented 3 years ago

Sure, I can take a look. I have another idea - move the example server/client to test and make their dependency as test scope - thus not leaking to the consumer classpath

demoth commented 3 years ago

@tlf30 Now test client and server works

tlf30 commented 3 years ago

It is generally considered bad practice to mix examples and test. Example code should be left out of the test code, only code related to unit and integration testing. At the moment we do not have any unit/integration tests in the project, but I plan to move some of the existing tests from Outside into this repo. Also, please do not increment the version number, 0.1.0 is not yet released so it does not need to be change.

Thank you for working on this. ~Trevor

demoth commented 3 years ago

@tlf30 I can put examples to the gradle submodule

demoth commented 3 years ago

please check if the naming is fine or not

tlf30 commented 3 years ago

I'm on my phone right now, but that looks fantastic, thank you. I will do a proper review when I get to a computer.

tlf30 commented 3 years ago

This looks great, thank you for your contribution!