kazuyukitanimura commented on code in PR #1569:
URL: https://github.com/apache/datafusion-comet/pull/1569#discussion_r2012813465
##########
spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala:
##########
@@ -27,24 +27,24 @@ class CometSparkSessionExtensionsSuite extends
CometTestBase {
import CometSparkSessionExtensions._
- test("isCometEnabled") {
+ test("isCometLoaded") {
val conf = new SQLConf
conf.setConfString(CometConf.COMET_ENABLED.key, "false")
- assert(!isCometEnabled(conf))
+ assert(!isCometLoaded(conf))
Review Comment:
Thanks, I thought about it but since both version of `isCometEnabled` would
be still accessible, especially due to
```
import CometSparkSessionExtensions._
```
, this may cause future issues for using `isCometEnabled` like adding new
tests.
I am ok with `CometSparkSessionExtensions.isCometEnabled` but changing the
name entirely is more future proof? WDTY @parthchandra @comphead
Also `CometSparkSessionExtensions.isCometEnabled` is checking
`NativeBase.isLoaded`, so I think `isCometLoaded` makes sense
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
