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

#228: Declare dependency on json4s optional #245

Merged
merged 5 commits into from
Mar 2, 2023
Merged

#228: Declare dependency on json4s optional #245

merged 5 commits into from
Mar 2, 2023

Conversation

saeltz
Copy link
Member

@saeltz saeltz commented Mar 1, 2023

Declares the dependency on json4s as optional. Consumers of this library need to add the dependency to their project to be able to use the VersionedModelReader.fromJson and VersionedModelWriter.toJson.

Fixes #228.

Has the version number been increased?

  • Yes!

@saeltz saeltz marked this pull request as ready for review March 1, 2023 10:54
@houcros
Copy link
Collaborator

houcros commented Mar 1, 2023

Just came to my mind: would it be worth updating the README? Because in this new version, a user would have to add a dependency to scalapb-json4s in the build to use that functionality, right? And it would probably not be obvious at first.

@saeltz
Copy link
Member Author

saeltz commented Mar 2, 2023

Generally a good idea but where would you add it? The documentation doesn't mention JSON at all anywhere.

@houcros
Copy link
Collaborator

houcros commented Mar 2, 2023

Ah I see, in that case it's unlikely anyone will miss those JSON methods. I had thought they were part of the main API somehow.

@JonasAckermann
Copy link
Collaborator

Hmm hmm, I think not mentioning it would still be a footgun :-D

@saeltz saeltz merged commit 46010ee into master Mar 2, 2023
@saeltz saeltz deleted the optional branch March 2, 2023 12:37
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.

Proposal: Separate protobuf and json parsers
3 participants