Json bytestring #7477
Json bytestring #7477
Conversation
fendor
force-pushed
the
json-bytestring
branch
2 times, most recently
from
3c480f7 to
eed2566
Compare
| renderJson (JsonObject | ||
| [ ("key", JsonString "foo\"bar") | ||
| , ("key2", JsonString "baz")]) | ||
| @?= "{\"key\":\"foo\\\"bar\",\"key2\":\"baz\"}" |
There was a problem hiding this comment.
There's been no concrete discussion of standards. Don't worry about it for now, and we'll come up with one
There was a problem hiding this comment.
Right. We only have https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#conventions, as mentioned in PR template. I guess, for now, use whatever conventions exist in the snippet you edit, unless they are utterly insane (in which case, create a reformatting commit first).
| @@ -75,7 +75,7 @@ library | |||
|
|
|||
| if !impl(ghc >= 7.8) | |||
| -- semigroups depends on tagged. | |||
| build-depends: tagged >=0.8.6 && <0.9 | |||
| build-depends: tagged >=0.8.6 && <0.9, bytestring-builder >= 0.10.8 && <0.11 | |||
There was a problem hiding this comment.
From #6948 (comment)
This is wrong as is, but I'll fix it before merge. One can have older bytestring on newer GHC, though that is contrived corner-case. But Cabal should be exemplary in own definition :)
Does that mean we don't need this dependency? Ill try to remove it and see whether CI is green.
There was a problem hiding this comment.
Ok, I guess CI tells us that it is not fine. I still don't know why it is wrong though. @phadej Would you mind elaborating?
fendor
force-pushed
the
json-bytestring
branch
2 times, most recently
from
ea01556 to
4c34d78
Compare
|
Theoretically ready to merge, but we need to resolve #7477 (comment) before merging. |
fendor
force-pushed
the
json-bytestring
branch
3 times, most recently
from
e38a5e7 to
0b311c8
Compare
fendor
force-pushed
the
json-bytestring
branch
from
0b311c8 to
3abcee5
Compare
|
@emilypi, @Mikolaj This PR is waiting for a decision in #7477 (comment), do you have any insights, or should be just merge? |
|
@fendor the edge case seems like it'd go away as we drop GHC's in the next series of PRs. I say let's merge. |
Rebase of #6948
As a bonus, adds escaping fully conform with https://datatracker.ietf.org/doc/html/rfc8259#section-7 and tests