refactor(staking-cli): move skip-metadata-validation to command args

[sveitser]

Move --skip-metadata-validation from global flag to MetadataUriArgs, making it a command-specific flag that appears after the subcommand.

Changes:

• Move skip_metadata_validation field to MetadataUriArgs
• Refactor metadata_uri to Option with custom value parser
• Add Clap constraints: requires, conflicts_with, required_unless_present
• Move MetadataUriArgs to metadata.rs for better organization
• Add tests for expected error messaged.

Breaking change: --skip-metadata-validation must now appear after the subcommand instead of before it.

I think it's fine to make this breaking change now instead of doing it in a backwards compatible way. Only few people have used the CLI and we haven't built anything on top of this.

[sveitser]

/gemini review

[gemini-code-assist]

Summary of Changes

Hello @sveitser, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the staking-cli to improve the handling of metadata URI validation. It moves the --skip-metadata-validation flag to be command-specific, reorganizes code for better maintainability, and adds constraints to the CLI argument parsing. This change is considered breaking, as it requires users to adjust their command-line usage.

Highlights

• Flag Relocation: The --skip-metadata-validation flag has been moved from a global flag to a command-specific flag, requiring it to be placed after the subcommand.
• Code Reorganization: The MetadataUriArgs struct has been moved to metadata.rs for better code organization.
• Clap Constraints: Added Clap constraints such as requires, conflicts_with, and required_unless_present to improve command-line argument handling.
• Metadata URI Parsing: Refactored metadata_uri to Option<Url> with a custom value parser to enforce URL constraints.
• Test Coverage: Added tests to verify expected error messages and command behavior.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• staking-cli/README.md
  • Updated documentation to reflect the new location of the --skip-metadata-validation flag and provide usage examples.
• staking-cli/src/cli.rs
  • Modified the run function to use metadata_uri_args.skip_metadata_validation instead of config.skip_metadata_validation when validating the metadata URI.
• staking-cli/src/lib.rs
  • Removed the global skip_metadata_validation field from the Config struct.
  • Removed the MetadataUriArgs struct and related tests from this file.
• staking-cli/src/metadata.rs
  • Added the MetadataUriArgs struct with Clap attributes for command-line argument parsing.
  • Implemented a custom value parser parse_metadata_url to enforce the 2048 byte limit on metadata URIs.
  • Added tests for MetadataUriArgs to ensure correct parsing and validation of metadata URIs.
• staking-cli/tests/cli.rs
  • Updated tests to reflect the new location of the --skip-metadata-validation flag after the subcommand.
  • Added tests to verify the behavior of the CLI with the new Clap constraints, including conflicts and requirements for metadata URI arguments.
• staking-cli/tests/node_signatures.rs
  • Updated tests to reflect the new location of the --skip-metadata-validation flag after the subcommand.

Activity

• The pull request moves the --skip-metadata-validation flag from a global flag to a command-specific flag.
• The MetadataUriArgs struct is moved to metadata.rs for better organization.
• Clap constraints are added to improve command-line argument handling.
• The metadata_uri is refactored to Option<Url> with a custom value parser.
• Tests are added to verify expected error messages.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request is a great refactoring that moves the --skip-metadata-validation flag from a global option to a command-specific argument within MetadataUriArgs. This change significantly improves the CLI's usability and structure by making argument relationships more explicit and less error-prone, enforced by clap constraints. The code is well-organized, with logic moved to the appropriate modules, and the changes are thoroughly tested, including new tests for the CLI argument constraints. The breaking change is well-documented and justified. My feedback includes a couple of minor suggestions to enhance code robustness and test maintainability.

[gemini-code-assist]

Code Review

The pull request successfully refactors the --skip-metadata-validation flag, moving it from a global option to a command-specific argument within MetadataUriArgs. This change significantly improves the CLI's argument parsing logic by making the flag's applicability more explicit and enforcing logical constraints. The README.md documentation has been updated to reflect this breaking change, providing clear examples of the new usage. The core logic for handling metadata arguments, including a custom URL parser with a byte limit, has been cleanly moved to metadata.rs. The implementation of clap attributes such as required_unless_present, conflicts_with, and requires for the new argument structure is well-done, ensuring robust and intuitive CLI behavior. Furthermore, the extensive updates and additions to the test suite thoroughly validate these new argument parsing rules, which is excellent for maintaining code quality and preventing regressions.

[imabdulbasit]

where is this limit coming from?>

[sveitser]

Just something we found that should work for everyone, anything longer is probably a mistake and will likely start to not work with some systems.

[sveitser]

I'll add a comment

[sveitser]

https://stackoverflow.com/a/417184

[sveitser]

added a comment here: e76b867

[sveitser]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great refactoring that improves the CLI's usability by moving the --skip-metadata-validation flag to be a command-specific argument. The use of clap's validation features like requires and conflicts_with makes the CLI more robust and user-friendly. The code is well-organized, with the metadata-related arguments now cleanly separated into metadata.rs. The addition of comprehensive tests for the new argument constraints is also a big plus. I have one minor suggestion regarding a small redundancy in validation logic, but overall this is an excellent change.