-
Notifications
You must be signed in to change notification settings - Fork 31
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
fixing bug related to Issue #457 #469
fixing bug related to Issue #457 #469
Conversation
@JSv4 or @pjohnmeyer ...can one of you review please? |
On it @sachin-shrestha |
Thanks, @sachin-shrestha, this all looks good to me. Is this something you need merged? If not, I'd like to revisit our versioning tomorrow. We wanted to more formally document that for a while. Since this would technically be a breaking change - even though it fixes a bug - I want to make sure we either (a) version up as appropriate or (b) have a sane and documented policy for the versioning. @pjohnmeyer, would love your thoughts on this. FYI, I also opened a new issue #470 to add some structural validation checks to catch both of the issues you fixed here. There's no reason they should have made it through the test suite. |
@@ -10,6 +10,6 @@ | |||
"type": "string" | |||
} | |||
}, | |||
"required": ["stock_class_id"], | |||
"required": ["issuer_id"], |
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.
We should also improve the description for the actual issuer transaction to make it clearer how it's being used
Let's also have a sample here too in the sample transactions file. |
Added samples, and the description looked okay but I made a minor change |
Just noting for posterity that this was not a breaking change because this object did not exist in v1.1.0. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
ObjectType.schema.json is missing the enum TX_ISSUER_AUTHORIZED_SHARES_ADJUSTMENT, resulting in a failure when validating sample data.
IssuerTransaction.schema.json references required field stock_class_id when it's not in the properties, should be issuer_id