-
Notifications
You must be signed in to change notification settings - Fork 159
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
Correct the data type of certain fields to store the expected design values. #498
Correct the data type of certain fields to store the expected design values. #498
Conversation
bcf8eb4
to
47f43ee
Compare
|
cf1c199
to
12da1c3
Compare
a845f68
to
4103a92
Compare
4103a92
to
315413a
Compare
@overcat , I'm starting review, I've looked at stellar/xdrgen#163, and it seems ok but I asked for other reviewers on that one as I don't have much experience there. |
one small suggestion, could you update the docs for xdr generation in readme.md to reflect the latest command needed to run, I'm not sure it's current for the new soroban |
This command is already up to date, but if you want to generate the xdr classes in this PR, you need to merge the following four PRs. |
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.
the updates look fine, before merging can you respond to the comment about BigInteger conversions to longs here:
https://github.com/stellar/java-stellar-sdk/pull/498/files#r1285301854
thanks!
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.
the changes look good, one remaining discussion of overflow on datetime's from the xdr HyperIntegers - #498 (comment)
once that is resolved, we can merge?
Closes #494, Closes #499
#487 depends on this PR.
xdrgen-related PR is in stellar/xdrgen#163
This PR includes breaking changes.
The types of the following fields have changed.