keksmd commented on issue #973: URL: https://github.com/apache/incubator-graphar/issues/973#issuecomment-5601489865
Dependency check before I cut this slice, so the ordering is agreed rather than discovered in review. The writer's own sources need only modules that are already on `main`: `graphar-core`, `graphar-io-api`, `graphar-storage-api`. Nothing in `src/main` reaches for a reader or a format backend. The block is entirely in test scope. Of the eight test classes, six import `org.apache.graphar.reader`, and every one of them verifies a write by reading the bytes back through a real Parquet backend. So a writer PR today would either arrive with no meaningful verification, or drag in both `graphar-reader` and `graphar-io-parquet` — which is the dependent chain we have been avoiding. There is also a size problem worth naming: `GraphWriter` is ~1600 lines in one class. That is too much for a single reviewable slice regardless of dependencies, so I plan to split it — chunk placement and offset maintenance separately from the property/topology write paths. Proposed order, which I would like to agree on before opening anything: 1. `io-parquet` (#964) — the shared gate; every downstream module verifies through it. Blocked only on #971 and #972. 2. `reader` — needed by the writer's round-trip tests and by the validator. 3. `writer` (this issue), split as above. 4. `validator` (#974). @SemyonSinchenko does that order work for you, and would you rather see the writer split the way I described or along a different seam? Happy to reshape it before writing the PR rather than after. -- 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]
