paleolimbot commented on PR #556: URL: https://github.com/apache/sedona-db/pull/556#issuecomment-3808764234
> As you wait for a proper review, I'll suggest that you consider breaking this into multiple PRs. @pwrliang kindly split this one out from the larger parent PR at my request...you are absolutely correct that 88 files and 6000 lines is a huge diff and those are excellent suggestions on how to split that up further. I do plan to attempt reviewing this tomorrow; however, we do need to respect that this development is happening in a public repository with a community and in the future (or now, if Peter or other members of the community are blocked from participating because of this PR's size) we will need to have changes be incremental. We want this code in SedonaDB not just because it is awesome but because we want to be the place where future contributors add CUDS-accellerated spatial functions, and to do that we'll need to work in the open and incrementally. My idea with scoping this to only include code in `c/sedona-libgpuspatial` was that it isn't used in the rest of the engine (and won't be by default without very special opt-in build/runtime configurations for some time). Even though this is large, the consequences of missing details are low...this is a very hard thing to do and until the end-to-end is in place it is hard to even know that it worked. Big PRs are definitely not ideal but also sometimes we need to do hard things. GPU spatial joins are a very very hard thing! -- 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]
