imbajin commented on code in PR #2906: URL: https://github.com/apache/incubator-hugegraph/pull/2906#discussion_r2544825255
########## hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftReflectionUtil.java: ########## @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.pd.raft; + +import com.alipay.sofa.jraft.Node; +import com.alipay.sofa.jraft.ReplicatorGroup; +import com.alipay.sofa.jraft.core.Replicator; +import com.alipay.sofa.jraft.entity.PeerId; +import com.alipay.sofa.jraft.util.ThreadId; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j Review Comment: ⚠️ **重构消除重复代码** 将 RaftEngine 和 PartitionEngine 中的重复反射逻辑提取到 RaftReflectionUtil 工具类中，是一个很好的重构实践。这样可以： 1. 消除代码重复（DRY 原则） 2. 统一维护反射逻辑 3. 修复 bug 时只需修改一处 建议： 1. 考虑为 `RaftReflectionUtil` 添加单元测试，特别是测试异常场景（如字段不存在、访问权限问题等） 2. 考虑添加类级别的 Javadoc 说明这个工具类的用途和使用场景 ########## hugegraph-store/hg-store-core/pom.xml: ########## @@ -178,6 +178,12 @@ <artifactId>hg-store-client</artifactId> <scope>test</scope> </dependency> + <dependency> + <groupId>org.apache.hugegraph</groupId> + <artifactId>hg-pd-core</artifactId> Review Comment: ‼️ **模块依赖问题 - 需要架构评审** 在 hg-store-core 的 pom.xml 中添加了对 hg-pd-core 的编译时依赖： ```xml <dependency> <groupId>org.apache.hugegraph</groupId> <artifactId>hg-pd-core</artifactId> <version>1.7.0</version> <scope>compile</scope> </dependency> ``` **潜在问题：** 1. **循环依赖风险**：PartitionEngine 现在依赖 hg-pd-core 中的 RaftReflectionUtil。需要检查 hg-pd-core 是否依赖 hg-store-core，避免循环依赖 2. **模块职责不清**：RaftReflectionUtil 放在 hg-pd-core 中，但被 store 模块使用。考虑是否应该放在一个更通用的 common 模块中？ 3. **版本硬编码**：版本号 1.7.0 是硬编码的，建议使用 ${project.version} 或在父 pom 中统一管理 **建议：** - 评估是否应该将 RaftReflectionUtil 移到 hugegraph-common 或创建一个 raft-common 模块 - 确认这个新的依赖关系符合项目的模块架构设计 -- 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]
