cmd/abigen: refactor command line interface #19797
Conversation
|
I see that some of the changes from my PR are included in there. So it's dependent 😓 Ok, I'll be as quick as possible. |
rjl493456442
force-pushed
the
abigen-support-mixins-2
branch
from
39dc646 to
99e4988
Compare
|
@gballet rebased. ptal |
|
@gballet the only reason the use |
|
yeah, in any case it's consistent with the rest of the codebase, so it's a good change |
gballet
left a comment
gballet
left a comment
There was a problem hiding this comment.
LGTM overall, please address the two questions that I have. I'm not finished testing yet, I'm just giving you a head start while I complete the testing.
cmd/abigen/main.go
Outdated
There was a problem hiding this comment.
I know I've been telling you to reactivate LangObjC because Peter rejected my PR to remove it (it's alway's Peter's fault), this being said we should put a warning here that ObjC support is incomplete.
| } | ||
| var userdoc, devdoc interface{} | ||
| json.Unmarshal([]byte(info.Userdoc), &userdoc) | ||
| json.Unmarshal([]byte(info.Devdoc), &devdoc) |
There was a problem hiding this comment.
What's the reasoning behind no longer checking the error here? UserDoc indeed doesn't seem to be used over the codebase, and if that it the case then let's remove it (not preferred) or report if something is invalid.
There was a problem hiding this comment.
Because user can select which information should be included in the combined-json.
See the CLI description: --combined-json abi,asm,ast,bin,bin-runtime,compact-format,devdoc,hashes,interface,metadata,opcodes,srcmap,srcmap-runtime,userdoc
Since Userdoc and Devdoc are not necessary for binding generation, so that error shouldn't be returned for the field absense.
2 participants
This PR does three things:
combined-jsonflag so that user can use json file directlyABISTDIN mode should read ABI only.This PR is based on #19718.