Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make toJson more JSONish #123

Merged
merged 1 commit into from
Apr 11, 2018
Merged

Make toJson more JSONish #123

merged 1 commit into from
Apr 11, 2018

Conversation

tonicebrian
Copy link

@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
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
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
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
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
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