Skip to content

Conversation

@jadenPete
Copy link

Previously, during mixed compilation (compiling Scala and Java sources in the same target), instead of analyzing the compiled Java classes, we analyzed all classes in the output directory. Because we compile the Scala sources first, this meant Scala classes were being re-analyzed.

I'm not sure why some Scala classes were missing code attributes, but the errors should be fixed (and compilation should be faster) by not re-analyzing them to begin with.

Previously, during mixed compilation (compiling Scala and Java sources
in the same target), instead of analyzing the compiled Java classes, we
analyzed all classes in the output directory. Because we compile the
Scala sources first, this meant Scala classes were being re-analyzed.

I'm not sure why some Scala classes were missing code attributes, but
the errors should be fixed (and compilation should be faster) by not
re-analyzing them to begin with.
Copy link

@jjudd jjudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a few nits and one recommended change.

val classes = DirectoryClassFinder(classesOutputDirectory).classes
val result = classes.paths.toSet

classes.close()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anything bad happen if this close isn't called? If so, should this use automatic resource management, so close is called automatically?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the original code in package.scala it looks like we will want to call close.

Can we utilize Using here to just call close automagically?

val javaSources = normalizedSources.view.map(_.toString).filter(_.endsWith(".java")).toList

if (javaSources.nonEmpty) {
val classesBefore = findClassesInOutputDirectory()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: before and after what? I believe it's Java compilation, but being a bit more specific here may help.

val classFinder = new DirectoryClassFinder(outputDirectory)
val classes = classFinder.classes

try {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this try was only for the resource management, but I figured I'd ask to make sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants