sbt / sbt-remote-control

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

Fix all unit tests #238

Closed jsuereth closed 9 years ago

jsuereth commented 9 years ago

Review by @havocp @eed3si9n

Notte, travis may failbecause of the 0.10.0-SNAPSHOT reference to pickling.

havocp commented 9 years ago

What's incompatible vs. 0.1 ? those tests were passing for me I thought.

jsuereth commented 9 years ago

If you never made it past "ModuleID" then they were not....

jsuereth commented 9 years ago

And I wrote what was incompatible. The way we encode FastTypeTag and the previous mechanism would need a bridge. Not really worth the effort IMHO.

havocp commented 9 years ago

re: protocol 0.2, if we don't care about 0.1 we could just overwrite it, which would have the nice side effect of showing in the diff how we are changing the protocol. Though there would be a lot of nonsense from reordering keys and adding ".0" to all the integers, perhaps.

The ".0" on integers I think is because we use JDouble always in JSONPickleFormat, see alternatives https://github.com/json4s/json4s/blob/3.3/ast/src/main/scala/org/json4s/JsonAST.scala#L185

jsuereth commented 9 years ago

Update with 0.2 protocol removed and 0.1 replaced

havocp commented 9 years ago

I raised a few questions but many of them predate this patch, the patch itself seems like a nice step forward. will leave it open in case @eed3si9n has comments I guess, but no objections to merging.

havocp commented 9 years ago

I guess this needs to be synced up with your pickling changes? Any way we can publish your pickling branch in a shared location with version, to develop against?

jsuereth commented 9 years ago

working on that now. I'm creating a giant merged PR w/ all three PR changes