[GitHub] [incubator-druid] gianm commented on issue #7285: Fix concurrency of VersionedIntervalTimeline

2019-03-18 Thread GitBox
gianm commented on issue #7285: Fix concurrency of VersionedIntervalTimeline URL: https://github.com/apache/incubator-druid/issues/7285#issuecomment-474102354 I don't really love that solution. Adding "InTest" seems to me like it would be pretty ugly, especially in the 200+ places that we

[GitHub] [incubator-druid] gianm commented on issue #7285: Fix concurrency of VersionedIntervalTimeline

2019-03-18 Thread GitBox
gianm commented on issue #7285: Fix concurrency of VersionedIntervalTimeline URL: https://github.com/apache/incubator-druid/issues/7285#issuecomment-474095468 In that case, maybe I misunderstand what `@VisibleForTesting` is for. Personally I've always used it to mean "would be private, but

[GitHub] [incubator-druid] gianm commented on issue #7285: Fix concurrency of VersionedIntervalTimeline

2019-03-18 Thread GitBox
gianm commented on issue #7285: Fix concurrency of VersionedIntervalTimeline URL: https://github.com/apache/incubator-druid/issues/7285#issuecomment-474081070 The method is marked `@VisibleForTesting`, which is like a way of saying "should be private, but has to be public so tests can use