Unfortunately, the last minute attendance of the meetup today threw my schedule off and I won't be able to look at this for at least a few days. Don't feel obligated to hold up for me.
On Wed, Jan 19, 2022, 9:04 AM Jacques Nadeau <jacq...@apache.org> wrote: > FYI, I'm trying to do a thorough review today (as much as possible with > patch this size). > > On Wed, Jan 19, 2022, 4:36 AM Michael Mior <mm...@apache.org> wrote: > >> Thanks for doing this Julian! I haven't been able to do a detailed review, >> but from my skim of the PR, this looks like a nice improvement. I think >> it's always been a bit difficult for new Calcite developers to write good >> tests, especially for new modules. So anything that helps that experience >> is very welcome. >> >> -- >> Michael Mior >> mm...@apache.org >> >> >> Le mer. 17 nov. 2021 à 01:15, Vladimir Sitnikov < >> sitnikov.vladi...@gmail.com> >> a écrit : >> >> > Jacques>This sounds like it will mean we will need to make calcite-core >> > test artifacts available >> > >> > Test artifacts publication is yet another anti-pattern just like "base >> test >> > class". >> > This change has been discussed: >> > https://lists.apache.org/thread/fz96p94h016p11g777otqntjxg2oxgh1 >> > >> > If you want to depend on a class from tests, consider moving it to >> /testkit >> > module: >> > >> > >> https://github.com/apache/calcite/tree/0899e6c157632ba1c5369a942cfe2be15fb4ed9f/testkit >> > >> > Jacques>We should think about the rules around Kotlin >> > >> > What happens in calcite-core/tests stays in calcite-core/tests :) >> > >> > It is reasonable to assume that testkit module would have dependencies, >> > and testkit would provide API that is usable from Java and other JVM >> > languages. >> > >> > In that regard, Kotlin dependency in testkit is not much different from >> > Quidem or commons-lang3. >> > Consumers might use Quidem if it fits just like they could use Kotlin >> if it >> > fits. >> > >> > Vladimir >> > >> >
