clintropolis commented on a change in pull request #6971: Added checkstyle for
"Methods starting with Capital Letters"
URL: https://github.com/apache/incubator-druid/pull/6971#discussion_r253555523
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -133,15 +133,15 @@ public void testSelectCountStart() throws Exception
CalciteTests.REGULAR_USER_AUTH_RESULT,
ImmutableList.of(Druids.newTimeseriesQueryBuilder()
.dataSource(CalciteTests.DATASOURCE1)
- .intervals(QSS(Filtration.eternity()))
- .filters(SELECTOR("dim2", "0", null))
+ .intervals(qss(Filtration.eternity()))
Review comment:
No dispute here that those methods names are not great, but I guess I can
see why they were done initially that way since those tests make big walls of
text (not an argument to keep them).
Should we just rename them to sensible names that spell out the type they
are making in this PR instead of renaming them to be lowercase and then having
to rename them all again later?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
