fix: use %q instead of manual \"%s\" #1723
Conversation
Noticed in a post-codereview of PR quicksilver-zone#1704 that there was a manual value quoting usinq `\"%s\"` instead of the format specifier `%q`. Manual quoting doesn't work correctly. One can find vestiges of it by git grep '\\"%s' Updates quicksilver-zone#1704
|
@odeke-em is attempting to deploy a commit to the quicksilver Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request focus on improving error message formatting across several methods in the interchain staking and participation rewards modules. Specifically, the error messages have been updated to use backticks for string formatting instead of double quotes. Additionally, the logic flow remains intact, with minor adjustments made to enhance clarity in error handling and validation processes. The modifications are primarily cosmetic, aimed at maintaining consistency and readability in error reporting without altering the core functionality. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- x/interchainstaking/keeper/msg_server.go (4 hunks)
- x/interchainstaking/keeper/msg_server_test.go (11 hunks)
- x/interchainstaking/keeper/proposal_handler.go (1 hunks)
- x/participationrewards/keeper/msg_server.go (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- x/interchainstaking/keeper/msg_server_test.go
🔇 Additional comments (6)
x/participationrewards/keeper/msg_server.go (1)
39-39: Approved: Improved error message formattingThe change from
\"%s\"to%qfor formatting themsg.Zonein the error message is a good improvement. It's more idiomatic in Go and aligns with the PR objectives.To ensure consistency across the codebase, let's check for any remaining instances of manual quoting:
If any results are found, consider updating them similarly for consistency.
✅ Verification successful
Verification Complete: No Manual Quoting Issues in Internal Codebase
The shell scripts identified instances of manual quoting in the following files:
third-party-chains/osmosis-types/osmoutils/accum/prefix.goThese instances are located within external dependencies and do not impact the internal codebase. The approved change to use
%qfor error message formatting maintains consistency within our codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for remaining instances of manual quoting in error messages # Test: Look for remaining instances of '\\"%s"' in Go files rg --type go '\\"%s"' # Test: Look for other potential manual quoting patterns rg --type go '"\s*%s\s*"'Length of output: 316
x/interchainstaking/keeper/msg_server.go (5)
101-101: Improved error message formattingThe changes from
\"%s\"to%qin the error messages are consistent with the PR objective and improve readability. This modification enhances the overall code quality without altering the function's logic.Also applies to: 104-104, 109-109
148-148: Consistent improvement in error message formattingThe changes from
\"%s\"to%qin the error messages align with the PR objective and enhance readability. This modification maintains consistency across the codebase and improves the overall quality of error reporting.Also applies to: 151-151, 155-155
211-211: Continued improvement in error message formattingThe change from
\"%s\"to%qin the error message is in line with the PR objective. This modification enhances readability and maintains consistency with the changes made in other functions.
247-247: Consistent error message formatting improvementThe change from
\"%s\"to%qin the error message aligns with the PR objective. This modification maintains the consistency of error message formatting throughout the file, enhancing overall code quality.
Line range hint
1-500: Summary of changes and their impactThe changes in this PR consistently replace
\"%s\"with%qin error messages across multiple functions (CancelRedemption,RequeueRedemption,UpdateRedemption, andSignalIntent). These modifications improve the readability and consistency of error messages throughout the file.Key points:
- The changes align with the PR objective.
- No logical modifications or potential issues have been introduced.
- The overall code quality has been enhanced through consistent error message formatting.
These changes contribute to a more maintainable and readable codebase.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1723 +/- ##
=======================================
Coverage 63.41% 63.41%
=======================================
Files 194 194
Lines 13436 13436
=======================================
Hits 8521 8521
Misses 4097 4097
Partials 818 818
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@odeke-em can you please just rebase the branch the fix for linting issue is already merged, thank you? |
Noticed in a post-codereview of PR #1704 that there was a manual value quoting usinq
\"%s\"insteadof the format specifier
%q. Manual quoting doesn't work correctly. One can find vestiges of it byUpdates #1704
Summary by CodeRabbit