-
Notifications
You must be signed in to change notification settings - Fork 56
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
Update swagger and proto-wire #232
base: master
Are you sure you want to change the base?
Update swagger and proto-wire #232
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self review
"{\"properties\":{\"wrapper\":{\"format\":\"int64\",\"maximum\":9223372036854775807,\"minimum\":-9223372036854775808,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
, expectSchema @TestProtoWrappers.TestUInt64Value | ||
"{\"properties\":{\"wrapper\":{\"maximum\":18446744073709551615,\"format\":\"UInt64Value\",\"minimum\":0,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
"{\"properties\":{\"wrapper\":{\"maximum\":18446744073709551615,\"minimum\":0,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
"{\"properties\":{\"wrapper\":{\"format\":\"int64\",\"maximum\":18446744073709551615,\"minimum\":0,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
, expectSchema @TestProtoWrappers.TestInt32Value | ||
"{\"properties\":{\"wrapper\":{\"maximum\":2147483647,\"format\":\"Int32Value\",\"minimum\":-2147483648,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
"{\"properties\":{\"wrapper\":{\"maximum\":2147483647,\"format\":\"int32\",\"minimum\":-2147483648,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
"{\"properties\":{\"wrapper\":{\"format\":\"int32\",\"maximum\":2147483647,\"minimum\":-2147483648,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
, expectSchema @TestProtoWrappers.TestUInt32Value | ||
"{\"properties\":{\"wrapper\":{\"maximum\":4294967295,\"format\":\"UInt32Value\",\"minimum\":0,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
"{\"properties\":{\"wrapper\":{\"maximum\":4294967295,\"minimum\":0,\"type\":\"integer\"}},\"type\":\"object\"}" | ||
"{\"properties\":{\"wrapper\":{\"format\":\"int32\",\"maximum\":4294967295,\"minimum\":0,\"type\":\"integer\"}},\"type\":\"object\"}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New swagger seems to sort keys alphabetically now.
instance Arbitrary TS.ShortText where | ||
arbitrary = fmap TS.fromText arbitrary | ||
shrink = map TS.fromText . shrink . TS.toText | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arbitrary instance for ShortText
already provided by quickcheck-instances
, this is the reason I increased the lower bound on that dep.
proto3-suite.cabal
Outdated
@@ -67,7 +67,7 @@ library | |||
|
|||
if flag(large-records) | |||
-- large-records support uses newer Dhall APIs. So we need at least 1.34. | |||
build-depends: dhall >=1.34 && < 1.39, | |||
build-depends: dhall >=1.34 && < 1.41.2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swagger2 >= 2.8 requires aeson 2, this conflicted with dhall <1.39 which requires pre-2 aeson. Bumped the upper bound to allow the solver to grab aeson 2.
packages: ./*.cabal | ||
source-repository-package | ||
type: git | ||
location: https://github.com/awakesecurity/proto3-wire | ||
tag: 8096a1e19431af603655e5c1f2488b5db907836c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stack was already using a commit pin for proto3-wire, added one for cabal. Also updated the pin in stack (this provides a Data
instance for FieldNumber
)
@riz0id Could I get a review when you have a moment? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JonathanLorimer thanks for the PR. Sorry it took so long to get around to reviewing it.
@JonathanLorimer it looks like there's a CI failure:
|
This update makes proto3-suite work as part of haskellPackages in unstable nixpkgs.