necouchman commented on PR #926: URL: https://github.com/apache/guacamole-client/pull/926#issuecomment-1788935762
@LemonZuo : Thanks for jumping in a contributing, here. A couple of notes about this: * I'm not sure there is an actual issue - if you have the `RemoteIpValve` configured correctly, the remote IP shows up correctly (at least, for me). The cases where multiple IP addresses are present or there are issues retrieving it usually have to do with situations where the `RemoteIpValve` is not 100% configured correctly. * I'm not sure that this warrants an actual completely new class? Maybe just a `private` method within the existing `Credentials` class would be just as effective, since it's only ever used there? I understand why you'd go that direction, particularly if it could be useful somewhere else, but seems like it may be best just to start it out within the `Credentials` class. * All changes need a Jira issue associated with them (https://issues.apache.org/jira/projects/GUACAMOLE) * Your code style needs to follow established style so that it is consistent with other code. https://guacamole.apache.org/open-source/. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
