imbajin commented on code in PR #687:
URL:
https://github.com/apache/incubator-hugegraph-toolchain/pull/687#discussion_r2480387367
##########
.github/workflows/loader-ci.yml:
##########
@@ -27,8 +27,8 @@ jobs:
TRAVIS_DIR: hugegraph-loader/assembly/travis
STATIC_DIR: hugegraph-loader/assembly/static
# TODO: replace it with the (latest - n) commit id (n >= 15)
- # hugegraph commit date: 2024-12-09
- COMMIT_ID: f838897
+ # hugegraph commit date: 2025-10-30
Review Comment:
‼️ **代码错误 - 日期错误**
注释中的日期 `2025-10-30` 是不正确的。这是一个未来的日期(2025年10月30日),但今天是
2025年10月31日。即使按照今天的日期,这个日期也不合理。
应该修正为实际的提交日期。请检查 hugegraph 仓库的 commit `5b3d295` 的真实提交日期。
##########
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/HugeGraphLoader.java:
##########
@@ -113,7 +113,6 @@ public static void main(String[] args) {
loader = new HugeGraphLoader(args);
} catch (Throwable e) {
Printer.printError("Failed to start loading", e);
Review Comment:
‼️ **代码逻辑问题 - 移除 System.exit(1) 可能导致测试行为不一致**
移除 `System.exit(1)` 调用会改变程序的错误处理行为:
**问题:**
- 原代码在加载失败时会立即终止 JVM 进程(退出码 1)
- 新代码只是 return,程序会继续执行
**影响:**
- 在单元测试中,这可能导致测试无法正确检测失败情况
- 在生产环境中可能导致错误被静默忽略
**建议:**
如果这个改动是为了改善测试体验,建议:
1. 考虑通过抛出异常而不是静默返回
2. 或者添加注释说明为什么移除 System.exit(1)
3. 确保所有调用方都能正确处理这种情况
--
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]
