imply-cheddar commented on code in PR #12715:
URL: https://github.com/apache/druid/pull/12715#discussion_r913295596
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java:
##########
@@ -234,7 +242,11 @@ private static Program buildHepProgram(Iterable<? extends
RelOptRule> rules,
return Programs.of(builder.build(), noDag, metadataProvider);
}
- private static List<RelOptRule> druidConventionRuleSet(final PlannerContext
plannerContext)
+ @VisibleForTesting
Review Comment:
The thing is, that's true for all code everywhere. Whether it's
public/private/package-private, it's up to the person who is using something to
determine why it's a good idea to use it. More often than not, games with
accessors like this only add friction to writing code and don't really add too
much value (it's not like the `@VisibleForTesting` annotation blocks the build
if it's used outside of tests or otherwise restricts type-ahead for IntelliJ).
Generally speaking, any place where this annotation needs to be added is
actually indicative of a problem with code structure and encapsulation as it
indicates that it's not possible to adequately test the class based on the
interfaces that are supposed to be interacted with.
--
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]
