imbajin commented on code in PR #2944:
URL:
https://github.com/apache/incubator-hugegraph/pull/2944#discussion_r2724268236
##########
hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/config/HugeConfigTest.java:
##########
@@ -32,6 +32,15 @@
import java.util.List;
import java.util.Map;
+import org.apache.commons.collections.IteratorUtils;
Review Comment:
**🧹 Minor: Import Order Inconsistency**
The test file has imports reordered (Apache Commons imports moved before
HugeGraph imports). While functionally correct, this breaks the existing import
ordering convention:
```diff
-import org.apache.hugegraph.config.ConfigConvOption;
-...
import org.apache.commons.collections.IteratorUtils;
import org.apache.commons.configuration2.AbstractConfiguration;
+...
+import org.apache.hugegraph.config.ConfigConvOption;
```
Consider keeping the original import order to reduce diff noise and maintain
consistency.
##########
hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/config/HugeConfig.java:
##########
@@ -213,4 +217,55 @@ private static Configuration loadConfigFile(File
configFile) {
e, configFile);
}
}
+
+ private static Object normalizeUrlOptionIfNeeded(String key, Object value)
{
+ if (value == null) {
+ return null;
+ }
+
+ String scheme = defaultSchemeFor(key);
+ if (scheme == null) {
+ return value;
+ }
+
+ // URL options are defined as ConfigOption<String> and normalized here.
Review Comment:
**🧹 Minor: Documentation Could Be Clearer**
The comment mentions "URL options are defined as ConfigOption<String>", but
this is an implementation detail that might confuse readers. Consider
clarifying the intent:
```suggestion
// Normalize URL config values by adding default scheme if missing.
// Only applies to allowlisted URL options (see defaultSchemeFor()).
if (value instanceof String) {
return prefixSchemeIfMissing((String) value, scheme);
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
