-
Notifications
You must be signed in to change notification settings - Fork 6k
Update EIP-7896: Fix JSON syntax in EIP-7896 ABI example #10806
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
base: master
Are you sure you want to change the base?
Conversation
File
|
| "version": "abi-v1", | ||
| "spec": [ | ||
| { | ||
| "type": "function" |
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.
Why is this redundant?
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 type field defaults to "function" per the ABI spec, so it's redundant when the item is a function. I removed it to simplify the example, but happy to restore it if you think explicit is better here.
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.
Please add it back for clarity.
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.
Restored "type": "function" in the example.
|
will let the author take a call on this first |
g11tech
left a comment
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.
lgtm for just dropping the trailing , to make it a valid json
|
Superceded by #10871 as there was another error in JSON that wasn't fixed in this PR. |
drop the trailing comma after the
"outputs": []array so the snippet becomes valid JSON