sbt / sbt-remote-control

Create and manage sbt process using unicorns and forks
Other
74 stars 14 forks source link

Add internal build information (build classpath, implicit imports) to MinimalBuildStructure #293

Closed nkly closed 9 years ago

jsuereth commented 9 years ago

This LGTM, is there anything else we may need in here? (cc @eed3si9n )

eed3si9n commented 9 years ago

I pulled the PR down to my local machine to play around.

scala> import sbt.serialization._
import sbt.serialization._

scala> import sbt.protocol.MinimalBuildStructure
import sbt.protocol.MinimalBuildStructure

scala> MinimalBuildStructure(Vector(), Vector(), Vector())
res0: sbt.protocol.MinimalBuildStructure = MinimalBuildStructure(Vector(),Vector(),Vector())

scala> toJsonString(res0)
res1: String = {"builds":[],"buildsData":[],"projects":[],"$type":"sbt.protocol.MinimalBuildStructure"}

scala> val protocol2Data = """{"builds":["file:\/\/\/test\/project"],"projects":[{"id":{"build":"file:\/\/\/test\/project","name":"test"},"plugins":["com.foo.Plugin"],"dependencies":null,"$type":"sbt.protocol.MinimalProjectStructure"}],"$type":"sbt.protocol.MinimalBuildStructure"}"""
protocol2Data: String = {"builds":["file:\/\/\/test\/project"],"projects":[{"id":{"build":"file:\/\/\/test\/project","name":"test"},"plugins":["com.foo.Plugin"],"dependencies":null,"$type":"sbt.protocol.MinimalProjectStructure"}],"$type":"sbt.protocol.MinimalBuildStructure"}

scala> fromJsonString[MinimalBuildStructure](protocol2Data)
res2: scala.util.Try[sbt.protocol.MinimalBuildStructure] = Success(MinimalBuildStructure(Vector(file:///test/project),Vector(),Vector(MinimalProjectStructure(ProjectReference(file:///test/project,test),Vector(com.foo.Plugin),None))))

Generally I'm happy to see that protocol2 data is deserializable using the new case class. I'm concerned that the newly added buildsData is missing from one-way test data, as commented.

nkly commented 9 years ago

@eed3si9n I've fixed things you've mentioned and updated protocol v3 tests for build structure change event as well.

eed3si9n commented 9 years ago

@dancingrobot84 Thanks. LGTM