-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
cfn command: TUI for listing up cloudformation stack #78
Conversation
WalkthroughThe recent updates introduce a significant enhancement to the user interface for managing AWS CloudFormation stacks. By incorporating a new interactive UI, users can now navigate, select, and perform actions on CloudFormation stacks with ease. The changes include the addition of color-coded stack statuses, the removal of redundant functions, and the introduction of new commands for fetching and listing stacks. Additionally, consistency improvements in naming conventions enhance the clarity of the codebase. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
startIndex = len(m.stacks) - windowHeight | ||
endIndex = len(m.stacks) | ||
} | ||
} else { |
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.
🚫 [golangci] reported by reviewdog 🐶
elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic)
return ui.ErrorMessage(m.err) | ||
} | ||
|
||
switch m.status { |
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.
🚫 [golangci] reported by reviewdog 🐶
missing cases in switch of type cfn.status: cfn.statusRegionSelecting, cfn.statusStacksListed, cfn.statusReturnToTop (exhaustive)
This comment has been minimized.
This comment has been minimized.
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.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files selected for processing (9)
- app/domain/model/cloudformation.go (2 hunks)
- cmd/subcmd/cfn/interactive.go (1 hunks)
- cmd/subcmd/cfn/ls.go (1 hunks)
- cmd/subcmd/cfn/root.go (1 hunks)
- ui/cfn/command.go (1 hunks)
- ui/cfn/list.go (1 hunks)
- ui/cfn/root.go (1 hunks)
- ui/cfn/status.go (1 hunks)
- ui/s3hub/list.go (1 hunks)
Additional comments: 6
cmd/subcmd/cfn/root.go (1)
- 12-14: The addition of conditional logic to trigger the interactive UI based on the command-line arguments is correctly implemented and enhances the user experience by providing an intuitive fallback to interactive mode when no arguments are provided. Ensure that this behavior change is clearly documented in the user help messages and documentation.
ui/cfn/status.go (1)
- 6-21: The defined statuses for the CloudFormation operation within the TUI are comprehensive and well-structured, facilitating clear and manageable state transitions throughout the user interaction. This implementation follows best practices for state management in UI applications.
cmd/subcmd/cfn/ls.go (1)
- 55-61: The updates to directly use the
StringWithColor
method for stack statuses and to skip processing for stacks with nil names or deleted status are well-implemented, enhancing code readability and performance. Ensure that theStringWithColor
method has comprehensive test coverage, as it directly affects user output.ui/s3hub/list.go (2)
- 282-287: The function
bucketListStrWithCheckbox
was renamed tobucketListStrWithCheckBox
for consistency and clarity. This change aligns with best practices for naming conventions, enhancing readability and maintainability of the code.- 279-290: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The file
ui/s3hub/list.go
includes a comprehensive implementation for managing S3 buckets within a TUI. It's crucial to ensure that all external calls, especially those interacting with AWS services, properly handle errors to prevent the application from crashing unexpectedly. Additionally, considering the interactive nature of the TUI, performance optimizations, such as minimizing the frequency of AWS API calls, could enhance the user experience.app/domain/model/cloudformation.go (1)
- 6-6: The addition of the
github.com/fatih/color
library is appropriate for implementing colored output in the CLI. This enhances the visual feedback for users by color-coding the stack statuses, making them easier to distinguish.
func interactive() error { | ||
return tui.RunCfnUI() | ||
} |
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.
The implementation of the interactive
function is straightforward and adheres to best practices by keeping the logic simple and focused. However, consider adding error logging or more detailed error handling for the RunCfnUI
call to enhance debuggability and user experience.
func fetchStacksCmd(ctx context.Context, app *di.CFnApp, region model.Region) tea.Cmd { | ||
return tea.Cmd(func() tea.Msg { | ||
output, err := app.CFnStackLister.ListCFnStack(ctx, &usecase.CFnStackListerInput{ | ||
Region: region, | ||
}) | ||
if err != nil { | ||
return ui.ErrMsg(err) | ||
} | ||
return fetchStacks{ | ||
stacks: output.Stacks, | ||
} | ||
}) | ||
} |
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.
The fetchStacksCmd
function is well-implemented, adhering to the asynchronous command pattern of the bubbletea
framework and correctly handling errors by returning them as messages for the UI. Consider adding error logging within the lambda function for improved debuggability and error tracking.
func RunCfnUI() error { | ||
ctx := context.Background() | ||
profile := model.NewAWSProfile("") | ||
cfg, err := model.NewAWSConfig(ctx, profile, "") | ||
if err != nil { | ||
return err | ||
} | ||
_, err = tea.NewProgram(newCFnRootModel(profile, cfg)).Run() | ||
return err | ||
} |
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.
The RunCfnUI
function and the cfnRootModel
structure are well-implemented, providing a solid foundation for the TUI's root model. Consider enhancing error handling in the RunCfnUI
function with error logging or more detailed user feedback to improve the user experience and debuggability in case of initialization failures.
m.status = statusReturnToTop | ||
return newCFnRootModel(m.awsProfile, m.awsConfig), nil | ||
case "D": | ||
// TODO: implement delete stack |
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.
The implementation of the cfnListStackModel
and its associated logic for handling user interactions and displaying stack information is well-done. However, there's a TODO comment for implementing stack deletion, indicating an incomplete feature. Consider addressing this to complete the feature set.
Would you like me to help implement the stack deletion feature or open a GitHub issue to track this task?
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- app/domain/model/cloudformation.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/domain/model/cloudformation.go
HOTTEST report
Reported by hottest |
Code Metrics Report
Details | | main (5d074e7) | #78 (8db2e3b) | +/- |
|---------------------|----------------|---------------|-------|
- | Coverage | 22.2% | 21.0% | -1.2% |
| Files | 60 | 64 | +4 |
| Lines | 1893 | 2001 | +108 |
| Covered | 421 | 421 | 0 |
+ | Test Execution Time | 39s | 15s | -24s | Code coverage of files in pull request scope (0.3% → 0.2%)
Reported by octocov |
Summary by CodeRabbit
New Features
cfn
command, enhancing user interaction with CloudFormation stacks.Bug Fixes
DeleteComplete
or when stack names are not provided.Refactor
stackStatusString
function in favor of direct usage ofStringWithColor
method.bucketListStrWithCheckbox
tobucketListStrWithCheckBox
for improved consistency and clarity.Chores