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

Disable JSON support by default #250

Merged
merged 2 commits into from
Jul 20, 2016

Conversation

seank-img
Copy link
Contributor

No description provided.

@DavidAntliff
Copy link
Collaborator

Looks good, happy to merge, curious as to whether there's a reason for the change though?

@DavidAntliff DavidAntliff added this to the 0.2.2 milestone Jul 20, 2016
@DavidAntliff DavidAntliff self-assigned this Jul 20, 2016
@seank-img
Copy link
Contributor Author

JSON support is more of an optional thing - when do we ever use it day to day? Also it slows down the build ;)

@cdewbery
Copy link
Collaborator

I guess it's okay to disable it as long as it still gets tested ;)

@delmet delmet merged commit 20ecfd1 into ConnectivityFoundry:master Jul 20, 2016
@DavidAntliff
Copy link
Collaborator

DavidAntliff commented Jul 20, 2016

That would be my only concern really - OK to disable it, but "slowing down the build" probably isn't a valid reason as we'd need to test it anyway, otherwise we might as well take it out completely (and that seems a shame given we do support it).

So, although it's too late now, my suggestion would be to modify this PR to enable it during the build.

@delmet
Copy link
Collaborator

delmet commented Jul 20, 2016

We are only talking about not having it on by default. Which means one less unneeded dependency and bloat for most situations.

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.

4 participants