Thespica commented on issue #676: URL: https://github.com/apache/incubator-graphar/issues/676#issuecomment-2904023894
> While writing unit tests, I found three issues: > > 1. Extra `/` in path concatenation(e.g. `"vertex/person//id/"`) (minor). > 2. Missing `getVersion()` in GraphInfo, VertexInfo, and EdgeInfo (may affect usage). > 3. Parameter order mismatch in EdgeInfo constructor vs. EdgeYaml#toEdgeInfo (**this can lead to serious bugs**). > Specifically, In the constructor of EdgeInfo, the parameter srcChunkSize appears before chunkSize. However, in org.apache.graphar.info.yaml.EdgeYaml#toEdgeInfo, srcChunkSize is passed after chunkSize. 1. Extra / in path concatenation: Thanks for pointing this out (e.g., "vertex/person//id/"). I'm not entirely sure where this redundant / might have been introduced in the path concatenation. If you've pinpointed the exact location in the code, a pull request to fix this would be welcome! 2. Missing getVersion() in GraphInfo, VertexInfo, and EdgeInfo: The getVersion() method wasn't included because the original proto files for these information classes did not define a version field. If having a version is important for your use case, please feel free to open a new, separate issue to propose this enhancement. 3. Parameter order mismatch in EdgeInfo constructor vs. EdgeYaml#toEdgeInfo: You are absolutely right about this! The discrepancy in parameter order (srcChunkSize and chunkSize) between the EdgeInfo constructor and the org.apache.graphar.info.yaml.EdgeYaml#toEdgeInfo method is a serious concern and could indeed lead to bugs. This is a perfect example of why increasing our unit test coverage is so important. Thank you for catching this critical detail! -- 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: commits-unsubscr...@graphar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@graphar.apache.org For additional commands, e-mail: commits-h...@graphar.apache.org
