Skip to content
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

cmd/abigen: make one of --abi or --combined-json be required #31045

Conversation

cedrick-ah
Copy link
Contributor

This PR addresses issue #30768 , which highlights that running cmd/abigen/abigen --pkg my_package example.json (erroneously omitting the --abi flag) generates an empty binding, when it should fail explicitly.

Changes

--abi flag is now mandatory when running abigen command and fail explicitly if it is missing

If theses changes are good, I’d be happy to continue familiarizing myself with the project and contributing to it!

cmd/abigen/main.go Outdated Show resolved Hide resolved
@jwasinger jwasinger changed the title abigen: make command abi flag mandatory cmd/abigen: make one of --abi or --combined-json be required. make them mutually exclusive Jan 19, 2025
cmd/abigen/main.go Outdated Show resolved Hide resolved
@jwasinger jwasinger changed the title cmd/abigen: make one of --abi or --combined-json be required. make them mutually exclusive cmd/abigen: make one of --abi or --combined-json be required Jan 19, 2025
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MariusVanDerWijden MariusVanDerWijden merged commit cc814d6 into ethereum:master Jan 20, 2025
3 checks passed
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Jan 21, 2025
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Jan 21, 2025
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants