Fix incorrect short flag for --success in ledger-stats log#1006
Fix incorrect short flag for --success in ledger-stats log#1006beck042 wants to merge 1 commit intomagicblock-labs:masterfrom
Conversation
Description: In the documentation for the log `subcommand` in `ledger-stats`, the short flag for `--success` was specified as `-s`, which was misleading. Reason for the fix: In the code (`src/main.rs`), the short flag `-u` (short = `“u”`) is used for `--success`. At the same time, `-s` is reserved for `--start`, so using `-s` for `--success` created a conflict and was misinterpreted by `StructOpt`. Fix: The short flag for `--success` has been changed to `-u` in the README. The usage examples have been updated to match the code and avoid confusion.
📝 WalkthroughWalkthroughThe pull request updates the documentation for the ledger-stats tool by modifying a command-line flag definition. Specifically, the short form flag for displaying successful transactions is changed from ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/ledger-stats/README.md (1)
8-8: 🧹 Nitpick | 🔵 TrivialOptional: Consider demonstrating the short form flag in examples.
While the current examples using
--successare correct, adding at least one example with the short form-uwould help users discover this option and confirm it works as documented.📝 Optional: Add example with short form
For instance, you could update line 8 to demonstrate the short form:
-cargo run --bin ledger-stats -- log ./tools/ledger-stats/ledger/ --success +cargo run --bin ledger-stats -- log ./tools/ledger-stats/ledger/ -uOr add a comment showing both forms are equivalent:
cargo run --bin ledger-stats -- log ./tools/ledger-stats/ledger/ --success +# Or using the short form: cargo run --bin ledger-stats -- log ./tools/ledger-stats/ledger/ -uAlso applies to: 68-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/ledger-stats/README.md` at line 8, Update the README examples to demonstrate the short-form flag for the same option shown with --success: add at least one example command showing -u (the short form of --success) alongside or in place of the existing cargo run example so users can see both forms work (reference the existing example using --success and the equivalent -u short flag).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tools/ledger-stats/README.md`:
- Line 8: Update the README examples to demonstrate the short-form flag for the
same option shown with --success: add at least one example command showing -u
(the short form of --success) alongside or in place of the existing cargo run
example so users can see both forms work (reference the existing example using
--success and the equivalent -u short flag).
2 participants
Summary
In the documentation for the log
subcommandinledger-stats, the short flag for--successwas specified as-s, which was misleading.Reason for the fix:
In the code (
src/main.rs), the short flag-u(short =“u”) is used for--success.At the same time,
-sis reserved for--start, so using-sfor--successcreated a conflict and was misinterpreted byStructOpt.Fix:
The short flag for
--successhas been changed to-uin the README.The usage examples have been updated to match the code and avoid confusion.
Compatibility
Testing
Checklist
Summary by CodeRabbit