-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Fix pubkey-macro args #34410
Fix pubkey-macro args #34410
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #34410 +/- ##
=======================================
Coverage 81.8% 81.9%
=======================================
Files 819 819
Lines 221019 221019
=======================================
+ Hits 180975 181033 +58
+ Misses 40044 39986 -58 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I threw in a couple of ideas, but I'm also happy with what you have as well. While putting it inside parenthesis kind of "demotes" the information, the information is still present for readers
4d35067
to
a5e3397
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few more nits
Thanks for the quick reviews! |
Problem
In #34288, we discovered there are a bunch of uses of the
solana_cli::pubkey
macro that add unnecessary punctuation, which ends up goofy. Eg:Summary of Changes
Remove periods and spaces. In a couple cases, we include extra information about the address -- I made these into parentheticals, so the punctuation still works. Open to other suggestions, though, as I don't love how it demotes the information.Update macro to be a complete sentence and adjust args accordingly.
I also fixed one argument definition that was confusing
(edit) Updated example below to match new approach
Fixed pubkeys look like: