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

refactor: --version in the CLI #632

Merged
merged 4 commits into from
Sep 17, 2024
Merged

refactor: --version in the CLI #632

merged 4 commits into from
Sep 17, 2024

Conversation

0xaatif
Copy link
Contributor

@0xaatif 0xaatif commented Sep 17, 2024

Came across this in passing.

Problems with the old code:

  • We included the hash of the Rust compiler we used??
    • I suspect we wanted the commit hash of the codebase @temaniarpit27 as implementor or @atanmarko as feature requester to confirm.
  • Manually scanned std::env::args for --version rather than using clap's built-in facilities for this.
  • EVM_ARITHMETIZATION_PKG_VER was ignored, so I've removed it.
  • A single-function module, pet peeve of mine.

I've renamed CIRCUIT_VERSION to KERNEL_HASH, because that's what it is.

See also #451 and #424

@0xaatif 0xaatif self-assigned this Sep 17, 2024
@github-actions github-actions bot added the crate: zero_bin Anything related to the zero-bin subcrates. label Sep 17, 2024
@atanmarko
Copy link
Member

@0xaatif @temaniarpit27 used the manual parsing of the std::env::args because feature request was that --version works in the same way as the --help command - if you put it anywhere in the arguments, it will print the version and exit (for example, that is how the rustc behaves). Now it does not work that way.

cargo run --bin leader -- clean --version
error: unexpected argument '--version' found

In this case maybe we could put subcommand for version (for the leader same level as clean, stdio etc. so that you run leader version not leader --version) - but for the others, not sure we have everywhere subcommand level. What do you think?

@0xaatif
Copy link
Contributor Author

0xaatif commented Sep 17, 2024

Thanks Marko,
I now understand why the previous code was written that way.
The correct fix is to use propogate_version, which I've done in ee93ec4.

Your example case now works.

@0xaatif 0xaatif merged commit eafbcc6 into develop Sep 17, 2024
17 checks passed
@0xaatif 0xaatif deleted the 0xaatif/version branch September 17, 2024 12:52
@Nashtare Nashtare added this to the System strengthening milestone Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: zero_bin Anything related to the zero-bin subcrates.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants