jeantil commented on a change in pull request #484:
URL: https://github.com/apache/james-project/pull/484#discussion_r647322652
##########
File path:
server/container/guice/configuration/src/main/java/org/apache/james/utils/PropertiesProvider.java
##########
@@ -43,11 +43,20 @@
import com.google.common.base.Strings;
public class PropertiesProvider {
-
private static final Logger LOGGER =
LoggerFactory.getLogger("org.apache.james.CONFIGURATION");
private static final char COMMA = ',';
private static final String COMMA_STRING = ",";
+ public static Configuration getConfiguration(File propertiesFile) throws
ConfigurationException {
Review comment:
:thinking: move from private instance method to public static ... ?
that's a huge scoping increase ...
I'm not saying it's wrong but it feels really weird and it seems to be done
exclusively for tests, isn't that a bit extreme ?
##########
File path:
server/container/guice/configuration/src/main/java/org/apache/james/utils/DelegatedPropertiesConfiguration.java
##########
@@ -372,6 +377,7 @@ public void unlock(LockMode mode) {
private Stream<String> splitAndStripDoubleQuotes(String value) {
return Optional.ofNullable(value)
+ .filter(s -> !StringUtils.isAllBlank(s))
Review comment:
shouldn't filtering come after
```
.map(String::trim) ?
```
instead ? to avoid leaving empty values such as `" "` flow through and be
converted to `""` ?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
