Yojson.Basic only, as my programs need to exchange data with programs written in other languages/using other JSON libraries/parsers.
I quite like the
Util module and use a handful of it’s functions.
To be fair I hadn’t but now that you mention it does seem sensible.
It has to be done the right way so that it doesn’t just mean twice as much maintenance and development work but besides that I’m pretty much sold on the idea.
That would also allow for a beta release of that Yojson successor with faster iterations. That would help figuring out exactly what its scope should be while still having a good release rate in the early stages of development. Hopefully that would help building a better library to which people would be happy to migrate to.
The problem I see with this is the second system syndrome, basically creating a split where nobody uses Yojson2 but everyone does use Yojson1 which is unmaintained, so you end up with a new version with little use and an old version with a lot of use. I would recommend against doing this unless you want to dedicate resources to maintain version 1 for a long time.
that might be a good idea indeed, sexps are simple and readable. I’ll see if I can find good combinators for decoding the config, not a big fan of ppxs for that.
We use Intlit, mainly because of int64 does not fit in Int.
We use Safe through atdgen and Basic wherever JSON is being used “manually”. I can pretty much guarantee that we’ll never use the Tuple or Variant types since we don’t want our implementation language to leak into the JSON we exchange.
It would be nice if we could make the basic type also safe, using some sort of bignum types for ints and floats, although I don’t know if there’s a good everyone-agrees bignum library for OCaml, and I don’t know if the performance hit is worth it.
I’m largely in favor of a new library name since dependency-hell is extremely common in OCaml, especially when making breaking changes in a central library like Yojson. We’ve had situations where we had to wait 6+ months between updating opam deps because it’s so difficult to find a combination of libraries that actually work together. Someone else mentioned the risk of having two libraries and people using the old one, but people could continue using the old version either way. The big difference is that one person staying on Yojson1 doesn’t prevent the rest of the universe from upgrading to Yojson2.
About the dependency hell problem, I’d like to make a remark about version bounds that were added at least on ppx_deriving_yojson. It looks like a bad idea and a problem to me (always easier to say afterwards). Now we are in a situation where some tools started to depend on the new type names (even though 2.0 is not released). Some can use any yojson version. And some are stuck on 1.5 for no good reason.
I understand that some users might have been annoyed by the new warnings. And that hiding those warnings is not always possible. But the choice the stick to 1.5 to avoid the warnings should be made by the users. Otherwise we end up in the current state. With code that would work fine with new versions of yojson. But can’t upgrade.
Also, I am wondering if introducing the new type names was really necessary. As anyway Safe, Base and Raw modules are supposed to be removed in yojson 2. Moving to Safe.t today doesn’t help with the future.
So I think that all library should stick to using
json instead of
t for now. Locally disable the deprecation warning where necessary. And stay compatible with most of the existing yojson and code out there.
Just to clarify, you’re suggesting that there shouldn’t be an upper bound on Yojson for old versions of ppx_deriving_yojson using the deprecated types right ?
I personally agree and I think deprecation warnings aren’t really well handled by the current tooling. If I knew how much of a mess it would’ve been I wouldn’t have deprecated the type but unfortunately it’s there now. I’m deeply sorry for the inconvenience.
Would a new release of ppx_deriving_yojson solve the issue for you ? I think there’s one underway, I’ll try to speed this up as much as I can but ppx_deriving_yojson maintainers have a lot on their plate and I’m on a holiday trip so that doesn’t help.
Exactly. Removing the < 1.6 limit that was added recently or turning it into < 2.0 instead.
I don’t really have an issue right now, it’s possible to work in the current situation. So I don’t feel an urgent need for a release. And I don’t want to disturb your holidays. The lack of contributors to ppx_deriving_yojson is indeed a problem.
My opinion is that https://github.com/ocaml-ppx/ppx_deriving_yojson/pull/90 should not have been merged, ppx_deriving_yojson should instead add annotations around the code it generates to locally disable the deprecation warning. But maybe I am missing something and it’s not possible.
Well, if more people were helping you maintaining the project, maybe the potential issues would have been spotted earlier. You are not to blame here, it’s already nice that you work on the project.
ppx_deriving_yojson needs redesing anyway (we need proper error messages at least instead of
Error: type), so yojson redesign would become a good motivation for this.
As the one who added the type, let me explain my reasoning:
I wanted to have
And since the convention is to use these names only if the type is called
t and I can never remember the arbitrary name of the type in
Yojson.Safe I thought it would be a good idea to have this type be called
t. In places where I use Yojson I keep on stumbling over these all the time so that’s why when Yojson moved into community-maintenance I decided to submit fixes for that are tripping me up personally.
Why didn’t I wait for unification of
Safe etc? Because at the time it was not on the table. I wanted to do a thing now and make it possible to adapt in my code soon, without having to redesign the library. You see the way it was added was backward-compatible apart from the deprecation. Ironically I’m still stuck on an older version of Yojson, since I also depend on
ppx_deriving_yojson whose latest release is not compatible with the new
t-type containing Yojson.
Overall I still stand by this, though marking the
safe type deprecated has unintentionally caused more issues than intended. Possibly @Khady is right and
ppx_deriving_yojson should be changed to instead silence the warning. Or
yojson 1.6.1 released which removes the deprecation (and reverts the
ppx_deriving_yojson change) to be added at a later point in time. I think both approaches are pretty simple to do.
In particular I have relatively little incentive to fix that module split (apart from pulling out
cppo of this whole thing) and forking Yojson to some other name makes it worse by having me to decide between the Yojson that has these annoying issues but is in use or Yojson2 that does not but also is not used by anyone. In the end I will probably end up with both in my dependency tree, which would be annoying.