gianm commented on PR #14866: URL: https://github.com/apache/druid/pull/14866#issuecomment-1686558352
> I actually did this initially, but then noticed that #14462 was also making this change, so reverted for now. That said, it would only be a minor conflict with the other PR, so Im willing to add it back. OK, fair enough. We can leave it for #14462. > Yeah, totally, i actually imagine a lot of other parts of IncrementalIndex could also be improved. Should I do that in this PR or save it for a follow-up? In the very least i can remove the group by v1 comment in addToFacts and add another comment to replace it indicating that the condition should now be impossible and we should work to simplify the code area in the future Up to you how much you want to do in this PR. I'd at least adjust the comment in there that mentions groupBy v1. Other stuff could be done as future work. -- 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]
