vlsi commented on PR #1963: URL: https://github.com/apache/calcite/pull/1963#issuecomment-6340988472
Thanks for working on this, @liyafan82, and sorry it sat for so long. I'm closing the PR because it can't be merged in its current form, though the goal is still valid: `DefaultEdge` on `main` still uses `Object` endpoints. - The branch is about 2700 commits behind `main` and conflicts in 10 of its 20 files, six of them from the nullability annotations added in CALCITE-4199. - The PR changes the bound of every graph implementation and iterator (`DefaultDirectedGraph`, `AttributedDirectedGraph`, `CycleDetector`, `TopologicalOrderIterator`, `Graphs`, and others) from `E extends DefaultEdge` to `E extends TypedEdge<V>`, and `DefaultDirectedGraph.create()` now returns `DefaultDirectedGraph<V, TypedEdge<V>>`. Since `DefaultEdge` is not a `TypedEdge`, user code that builds a `DefaultDirectedGraph<V, DefaultEdge>` or uses its own `DefaultEdge` subclass with these classes stops compiling. Julian suggested `TypedEdge` on JIRA as a separate class instead of changing `DefaultEdge`, but retargeting the graph classes to it breaks existing `DefaultEdge` users anyway. A new PR would need to keep `DefaultEdge`-based graphs compiling, for example by making `TypedEdge<V>` a subclass of `DefaultEdge` with typed `source()`/`target()` accessors and keeping the existing bounds. It would also need to pass the Checker Framework build on `main` (`equals(@Nullable Object)`) and keep the `toString()` added in CALCITE-4425. Feel free to open a new PR if you want to pick it up again. -- 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]
