imbajin commented on code in PR #302:
URL: 
https://github.com/apache/incubator-hugegraph-ai/pull/302#discussion_r2447061036


##########
hugegraph-llm/pyproject.toml:
##########
@@ -85,3 +86,4 @@ allow-direct-references = true
 
 [tool.uv.sources]
 hugegraph-python-client = { workspace = true }
+pycgraph = { git = "https://github.com/ChunelFeng/CGraph.git";, subdirectory = 
"python", rev = "248bfcfeddfa2bc23a1d585a3925c71189dba6cc"}

Review Comment:
   ‼️ **Critical: Missing dependency declaration**
   
   The project adds `pycgraph` dependency via git reference but doesn't 
document:
   1. Why this specific commit hash (`248bfcf`) is required
   2. What happens if this remote repository becomes unavailable
   3. Whether this should eventually be replaced with a versioned release
   
   **Recommendation**: Add a comment in `pyproject.toml` explaining the pinned 
commit, or better yet, work with upstream to create a tagged release.



##########
hugegraph-llm/pyproject.toml:
##########
@@ -17,7 +17,7 @@
 
 [project]
 name = "hugegraph-llm"
-version = "1.5.0"
+version = "1.7.0"

Review Comment:
   ⚠️ **Important: Version bump rationale missing**
   
   Version jumped from `1.5.0` to `1.7.0`, skipping `1.6.0`. This suggests 
either:
   - Major architectural changes (which aligns with this PR's scope)
   - Potential versioning mistake
   
   **Recommendation**: 
   - Document the version bump rationale in the PR description or CHANGELOG
   - If this is a breaking change (old RAGPipeline/KgBuilder removed), consider 
bumping to `2.0.0` per semantic versioning



-- 
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]

Reply via email to