Copilot commented on code in PR #12419:
URL: https://github.com/apache/gluten/pull/12419#discussion_r3507506676
##########
gluten-substrait/src/test/scala/org/apache/gluten/config/AllGlutenConfiguration.scala:
##########
@@ -228,19 +227,19 @@ object AllGlutenConfiguration {
*/
def assertFileContent(
path: Path,
- expectedLines: Traversable[String],
+ expectedLines: Seq[String],
regenScript: String,
splitFirstExpectedLine: Boolean = false)(implicit
prettifier: Prettifier,
pos: Position): Unit = {
val fileSource = Source.fromFile(path.toUri, StandardCharsets.UTF_8.name())
try {
def expectedLinesIter = if (splitFirstExpectedLine) {
- Source.fromString(expectedLines.head).getLines()
+ Source.fromString(expectedLines.head).getLines().toSeq
} else {
- expectedLines.toIterator
+ expectedLines
}
Review Comment:
`expectedLinesIter` / `fileLinesIter` are no longer iterators (they are
`Seq`s), and `expectedLinesIter` is a `def` that can recompute the sequence
multiple times (notably when `splitFirstExpectedLine=true`). Converting these
to `val`s with names that reflect the actual type will avoid repeated work and
reduce confusion when reading this assertion logic.
##########
pom.xml:
##########
@@ -549,6 +549,28 @@
</testResources>
<pluginManagement>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-enforcer-plugin</artifactId>
+ <executions>
+ <execution>
Review Comment:
`maven-enforcer-plugin` is now declared twice under
`<build><pluginManagement><plugins>`: this new block (without a `<version>`)
and an existing `maven-enforcer-plugin` entry later in the same `<plugins>`
list (with `<version>3.3.0</version>`). Keeping duplicate plugin definitions is
brittle because Maven’s model merge behavior can be non-obvious and may cause
the execution to be dropped/overridden; it’s safer to consolidate into a single
`maven-enforcer-plugin` definition that includes both the version and the new
`enforce-spark-profile` execution. Also, the inline comment “Spark 4.0 requires
Java 17+” is unrelated to enforcing that a Spark profile is selected; consider
updating it to describe the actual rule being enforced.
--
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]
