Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/68#discussion_r77440372
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPGuacamoleProperties.java
---
@@ -144,5 +144,8 @@ private LDAPGuacamoleProperties() {}
public String getName() { return "ldap-encryption-method"; }
};
-
+ public static final StringGuacamoleProperty
ADDITIONAL_LDAP_SEARCH_FILTERS = new StringGuacamoleProperty(){
+ @Override
+ public String getName() { return
"ldap-additional-search-filter"; }
--- End diff --
Again, please try to follow the style of the rest of the code. Consider
this existing property from earlier in this file:
```
/**
* The base DN to search for Guacamole configurations.
*/
public static final StringGuacamoleProperty LDAP_CONFIG_BASE_DN = new
StringGuacamoleProperty() {
@Override
public String getName() { return "ldap-config-base-dn"; }
};
```
There is a descriptive property name which is consistent between the Java
code (`LDAP_CONFIG_BASE_DN`) and the property that would be used in
guacamole.properties (`ldap-config-base-dn`), and the Java property is properly
documented with a JavaDoc comment.
Compare to:
```
public static final StringGuacamoleProperty
ADDITIONAL_LDAP_SEARCH_FILTERS = new StringGuacamoleProperty(){
@Override
public String getName() { return "ldap-additional-search-filter";
}
};
```
Note:
1. The JavaDoc comment is missing.
2. The Java naming (`ADDITIONAL_LDAP_SEARCH_FILTERS`) does not match that
of the property (`ldap-additional-search-filter`).
3. The spacing surrounding the `getName()` implementation with respect to
blank lines is inconsistent with the rest of the code in this file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
