chore: remove true comparision #1558
Conversation
WalkthroughThe recent updates involve simplifying and optimizing the code in two parts of the Go CLI application. The first change refactors the logic for determining file paths in the JWT commands, reducing redundancy and consolidating file path assignments. The second change enhances the Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- mod/cli/pkg/commands/jwt/jwt.go (1 hunks)
- mod/primitives/pkg/hex/nibble.go (1 hunks)
Additional comments not posted (4)
mod/primitives/pkg/hex/nibble.go (3)
31-31: Simplified condition for decoding '0' to '9'.The changes streamline the condition checks, making the code cleaner and potentially faster by directly comparing the character ranges.
33-33: Simplified condition for decoding 'A' to 'F'.This adjustment enhances readability and efficiency by focusing directly on the relevant character range.
35-35: Simplified condition for decoding 'a' to 'f'.Like the previous changes, this modification improves the clarity and directness of the condition, aligning with the PR's goal of removing redundant logic.
mod/cli/pkg/commands/jwt/jwt.go (1)
120-127: Refactored file path retrieval logic.The removal of the redundant check for an empty
specifiedFilePathand the consolidation of the file path assignment process simplify the function and reduce unnecessary branching. This change should make the function more straightforward and maintainable.
mod/cli/pkg/commands/jwt/jwt.go
Outdated
| } | ||
| specifiedFilePath = filepath.Join( | ||
| clientCtx.HomeDir+"/config/", DefaultSecretFileName, |
There was a problem hiding this comment.
nit: filepath.Join appends the backslashes correctly according to os spec so hardcoding /config/ isn't necessary if i remember correctly
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- mod/cli/pkg/commands/jwt/jwt.go (2 hunks)
Additional comments not posted (2)
mod/cli/pkg/commands/jwt/jwt.go (2)
38-38: Introduction ofConfigFolderconstant:The addition of the
ConfigFolderconstant enhances modularity by centralizing the configuration directory path. This change supports the PR's objective of simplifying and streamlining code.
121-128: Refactored default file path logic ingetFilePath:The refactoring of the default file path logic to use
clientCtx.HomeDircombined with the newConfigFolderconstant is a good improvement. It simplifies the path construction and makes the function more readable and maintainable.
| @@ -28,11 +28,11 @@ import ( | |||
| func decodeNibble(in byte) uint64 { | |||
| // uint64 conversion here is safe | |||
| switch { | |||
| case in >= '0' && in <= '9' && in >= hexBaseOffset: | |||
There was a problem hiding this comment.
hexBaseOffset is already '0' so no need for comparison here. Same with the other if statements
There was a problem hiding this comment.
you're right. wanna add a brief comment clarifying that this would never underflow?
There was a problem hiding this comment.
or maybe above each return, add a comment containing the expected range of the result.
ex. in - hexBaseOffset would result in a value in 0-9, in - hexAlphaOffsetUpper would result in a value in 10-15, etc.
| @@ -117,16 +118,14 @@ func getFilePath(cmd *cobra.Command, path string) (string, error) { | |||
| // If no path is specified, try to get the cosmos client context and use | |||
| // the configured home directory to write the secret to the default file | |||
| // name. | |||
| if specifiedFilePath == "" { | |||
There was a problem hiding this comment.
i don't think this is logically equivalent
There was a problem hiding this comment.
we already early return if specifiedFilePath is not empty here so no need to check again.
There was a problem hiding this comment.
yup I agree with all points,
@TropicalDog17 just add a comment for what ocnc2 and i'm good to merge
Description
Remove instances of redundant logical conditions that alway result in true.
Summary by CodeRabbit