-
Notifications
You must be signed in to change notification settings - Fork 33
IPv6 Integration #281
IPv6 Integration #281
Conversation
I will start adding it in just to see how it behaves and works. Later i will cleanup the code and customize on the new Worker. |
but one of them can be zero, exception is being thrown if both are zero. |
Someone know what's the Problem with NetworkConnection.java:157 in my fork. Help appreciated ^^ |
Agree, just overlooked it probably. |
Here is an overview of what got changed by this pull request: Issues
======
+ Solved 4
- Added 5
Complexity increasing per file
==============================
- cloudnet-wrapper/src/main/java/eu/cloudnetservice/cloudnet/v2/wrapper/server/BungeeCord.java 2
- cloudnet-wrapper/src/main/java/eu/cloudnetservice/cloudnet/v2/wrapper/CloudNetWrapperConfig.java 2
See the complete overview on Codacy |
CloudNetWrapper.getInstance().getWrapperConfig().getCloudnetPort())) | ||
.saveAsConfig(cloudPath.resolve("connection.json")); | ||
} else { | ||
throw new NullPointerException("No CloudNet host defined!"); |
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.
Codacy found an issue: Avoid throwing null pointer exceptions.
private Optional<String> cloudnet4Host = Optional.empty(); | ||
private Optional<String> cloudnet6Host = Optional.empty(); | ||
|
||
private String internalIP, wrapperId, proxyConfigHost; |
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.
Codacy found an issue: Use one line for each declaration, it enhances code readability.
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.
Isn't this against the code of conduct? Or got it updated?
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.
Optionals are not intended for fields
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.
And yes, we never do multi decalrations in one line
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.
Optionals are not intended for fields
better than always using the getter to get the optional.
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.
But this does not correspond to the code style of any kind. Please do it in the getter
CloudNetWrapper.getInstance().getWrapperConfig().getCloudnetPort())) | ||
.saveAsConfig(cloudPath.resolve("connection.json")); | ||
} else { | ||
throw new NullPointerException("No CloudNet host defined!"); |
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.
Codacy found an issue: Avoid throwing null pointer exceptions.
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.
Please create your own exception and use it correctly. Runtime Exception with the Logger
|
||
if (!host4Name.isPresent() && !host6Name.isPresent()) { | ||
//TODO: Exit or rerequest data | ||
System.exit(0); |
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.
Codacy found an issue: System.exit() should not be used in J2EE/JEE apps
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.
Best replace the exit with a call to a method. Since it may be here that the system does not shut down properly
@@ -69,18 +70,24 @@ public CloudNetWrapper(OptionSet optionSet, CloudNetWrapperConfig cloudNetWrappe | |||
|
|||
this.wrapperConfig = cloudNetWrapperConfig; | |||
this.cloudNetLogging = cloudNetLogging; | |||
|
|||
if (!cloudNetWrapperConfig.getCloudnetHost().isPresent()) { | |||
throw new NullPointerException("no cloudnet host defined!"); |
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.
Codacy found an issue: Avoid throwing null pointer exceptions.
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.
Please create your own exception and use it correctly. Runtime Exception with the Logger
Not sure if this is ready for a review? @Likqez can you please give a feedback? |
I will remove the WIP status if ready. Currently just not much time to contribute. |
Gotcha |
Is there any progress on this or should I incorporate the changes in this PR into another branch and take over the issue? |
There is no progress & no local changes left right now. I just can't find the time and motivation to progress. You could take over. |
Alright, I will merge this PR into a new branch and continue development there. |
In this pr, we're implementing dual stack support. For Ipv4 and 6.
Suggestions and help are welcome, @TheMeinerLP we can work together if you want(added you as collaborator.)
#275