Skip to content

Make toJson more JSONish#123

Merged
eed3si9n merged 1 commit intosbt:masterfrom
tonicebrian:master
Apr 11, 2018
Merged

Make toJson more JSONish#123
eed3si9n merged 1 commit intosbt:masterfrom
tonicebrian:master

Conversation

@tonicebrian
Copy link
Copy Markdown

@tonicebrian tonicebrian commented Apr 11, 2018

In all HTTP services I make, I add an endpoint /build outputting the toJson result of the BuildInfo. That output is not fully JSON and thus I lack the ability of manipulating output. This pull request is for making that output a bit more JSONish without going full way about adding an external dependency like akka-http-spray-json.

Now I'm able to compare libraries compiled in two instances by doing things like:

curl localhost:8900/build | jq ".libraryDependencies | sort"

Copy link
Copy Markdown
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

lgtm. it doesn't handle nested seqs/options, but that's probably not that common.

requesting an extra review by @eed3si9n

@dwijnand dwijnand requested a review from eed3si9n April 11, 2018 09:31
@eed3si9n
Copy link
Copy Markdown
Member

How does the before and after look like? Is this going to be a breaking change for existing users of this feature?

@dwijnand
Copy link
Copy Markdown
Member

there is a test (that changed). it tests that the code generator generates the right code. but there's no test for the JSON string value that results from running the generated code.

how metacan youget 😄

@tonicebrian
Copy link
Copy Markdown
Author

tonicebrian commented Apr 11, 2018

@eed3si9n Regarding breaking code for existing users, this PR is going to break code that parses libraryDependencies as a string

...
"libraryDependencies" : "scala.collection.Seq(org.scala-lang:scala-library:2.12.4, com.lihaoyi:acyclic:0.1.7:plugin->default(compile), .....)"
...

because now it will be:

...
"libraryDependencies" : ["org.scala-lang:scala-library:2.12.4", "com.lihaoyi:acyclic:0.1.7:plugin->default(compile)", .....]
...

I guess that if they were parsing the Json before, it is way easier to adapt the code so they have proper Json lists and options (to the first level of nesting as pointed by @dwijnand )

@eed3si9n
Copy link
Copy Markdown
Member

ok. I'll be happy to merge this, but we should bump the version number to 0.9.0.

@eed3si9n eed3si9n merged commit e1d2ace into sbt:master Apr 11, 2018
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