-
Notifications
You must be signed in to change notification settings - Fork 1
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
Cleanup the create account arguments so only including a profile is a valid path #297
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes modify the account creation logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as CLI Command
participant CMD as MakeMainCommand
participant Validator as Profile Checker
participant Account as Account Creation
User->>CLI: Run account creation command
CLI->>CMD: Invoke MakeMainCommand
CMD->>Validator: Check "create-account-profile" flag
alt Profile flag present and valid
Validator-->>CMD: Valid profile map returned
CMD->>CMD: Add "login" and "email" if missing
CMD->>Account: Create account with structured profile
Account-->>CMD: Account successfully created
else Profile flag missing/invalid
CMD->>Account: Process using fallback login/email flags
Account-->>CMD: Account created via fallback
end
CMD-->>CLI: Return account creation result
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/cli/commands.go (2)
131-133
: Improve error message clarity.The error message when the profile is incorrectly formatted could be more helpful by showing the expected format.
Apply this diff to enhance the error message:
if profileMap == nil { - return fmt.Errorf("create-account-profile is empty or incorrectly formatted: %v", v.GetString("create-account-profile")) + return fmt.Errorf("create-account-profile must be a valid JSON object, got: %v. Example format: '{\"login\":\"user\",\"email\":\"[email protected]\"}'", v.GetString("create-account-profile")) }
129-179
: Architecture aligns well with PR objectives.The implementation successfully:
- Makes profile-based account creation the primary path
- Merges login/email arguments into the profile when provided
- Maintains backward compatibility through the fallback path
This aligns perfectly with the PR's goal of cleaning up account creation arguments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/cli/commands.go
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: go-test (1.22.x, windows-latest)
🔇 Additional comments (1)
pkg/cli/commands.go (1)
163-179
: LGTM! Good fallback implementation.The fallback logic maintains backward compatibility while creating a profile structure, aligning well with the PR's objective to make profile-based creation the primary path.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/cli/commands.go (2)
131-133
: Improve error message for profile validation.The error message could be more helpful by providing guidance on the expected format.
- return fmt.Errorf("create-account-profile is empty or incorrectly formatted: %v", v.GetString("create-account-profile")) + return fmt.Errorf("create-account-profile must be a non-empty map in the format '{\"key\":\"value\"}', got: %v", v.GetString("create-account-profile"))
144-154
: Simplify login and email extraction.The nested type assertions can be simplified for better readability.
- login, email := "", "" - if l, ok := profileMap["login"]; ok { - if l, ok := l.(string); ok { - login = l - } - } - if e, ok := profileMap["email"]; ok { - if e, ok := e.(string); ok { - email = e - } - } + login := "" + if l, ok := profileMap["login"].(string); ok { + login = l + } + email := "" + if e, ok := profileMap["email"].(string); ok { + email = e + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/cli/commands.go
(2 hunks)
🔇 Additional comments (2)
pkg/cli/commands.go (2)
167-183
: LGTM! Good fallback implementation.The fallback logic correctly maintains backward compatibility by creating a profile from login and email when
create-account-profile
is not provided.
319-320
: LGTM! Consistent provisioning support.The changes correctly enable provisioning support for profile-based account creation in the GRPC server command.
We now check the profile first, we merge the extra login/email args into it if provided, and get login/email from there directly.
This lets us totally ignore the login/email args on the cli if desired.
We also create a profile from the email/login provided for the old flow.
Summary by CodeRabbit