feat: replace go-prompt dependency

danstis commented 8 months ago

User description

Fixes #196

Type

enhancement, bug_fix

Description

Replaced github.com/segmentio/go-prompt dependency with custom user confirmation logic using fmt.Scanln to improve control and reduce external dependencies.
Updated usage information display from flag.PrintDefaults() to flag.Usage().
Removed github.com/segmentio/go-prompt and its indirect dependencies from go.mod and go.sum.

Changes walkthrough

Relevant files

Enhancement

rmstale.go

Replace go-prompt with custom confirmation logic

rmstale.go

Replaced prompt.Confirm with custom confirmation logic using
fmt.Scanln.

Changed flag.PrintDefaults() to flag.Usage() for displaying command
usage.

+9/-3

Dependencies

go.mod `Remove go-prompt dependency from go.mod` go.mod - Removed `github.com/segmentio/go-prompt` dependency.	+0/-4
go.sum `Update go.sum after removing go-prompt` go.sum Updated `go.sum` to reflect removal of `github.com/segmentio/go-prompt` and its dependencies.	+0/-8

✨ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

sophie-syntax[bot] commented 8 months ago

PR Description updated to latest commit (https://github.com/danstis/rmstale/commit/7b61a6d4e888d5cc0bc1d16c99e1a3b928f70824)

sophie-syntax[bot] commented 8 months ago

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and localized to specific functionalities. The removal of an external dependency and its replacement with native code is clear, and the modifications in the go.mod and go.sum files are directly related to this change.
🧪 Relevant tests	No
🔍 Possible issues	User Input Validation: The new custom confirmation logic does not validate user input beyond checking for 'y' or 'Y'. Consider handling unexpected input more gracefully.
🔍 Possible issues	Error Handling: The error from `fmt.Scanln` is logged but does not prevent further execution. This might not be an issue in the current context but could lead to unintended behavior in more complex scenarios.
🔒 Security concerns	No

✨ Review tool usage guide:

**Overview:** The `review` tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be [added](https://pr-agent-docs.codium.ai/tools/review/#general-configurations) by configuring the tool. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on any PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L23) related to the review tool (`pr_reviewer` section), use the following template: ``` /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_reviewer] some_config1=... some_config2=... ``` See the review [usage page](https://pr-agent-docs.codium.ai/tools/review/) for a comprehensive guide on using this tool.

sophie-syntax[bot] commented 8 months ago

PR Code Suggestions

Category	Suggestions
Enhancement	Improve user input handling by allowing "yes" in addition to "y". ___ Consider handling the case where the user inputs "yes" or "Yes" in addition to "y" and "Y" for a more user-friendly experience. This can be done by converting the response to lowercase and then checking if it equals "y". [rmstale.go [84]](https://github.com/danstis/rmstale/pull/197/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR84-R84) ```diff -if response != "y" && response != "Y" { +if strings.ToLower(response) != "y" { ```
	Ensure consistent flag usage messaging by using `flag.PrintDefaults()`. ___ Replace `flag.Usage()` with `flag.PrintDefaults()` to ensure that the default usage message, including all flags, is printed. This change maintains consistency with the previous behavior. [rmstale.go [72]](https://github.com/danstis/rmstale/pull/197/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR72-R72) ```diff -flag.Usage() +flag.PrintDefaults() ```
	Add a brief sleep before reading user input to improve visibility of the warning message. ___ Consider adding a brief sleep after printing the warning message and before reading the user input. This can help ensure that the message is fully visible to the user before the program waits for input, improving user experience on some terminals. [rmstale.go [77-79]](https://github.com/danstis/rmstale/pull/197/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR77-R79) ```diff fmt.Printf("WARNING: Will remove files and folders recursively below '%v'%s older than %v days. Continue? (y/n) ", filepath.FromSlash(folder), extMsg, age) +time.Sleep(2 * time.Second) var response string _, err := fmt.Scanln(&response) ```
Best practice	Exit with a non-zero status code after logging an input read error. ___ After logging an error with `logger.Errorf` when failing to read user input, consider exiting with a non-zero status code to indicate an error condition to the calling environment. [rmstale.go [81-82]](https://github.com/danstis/rmstale/pull/197/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR81-R82) ```diff logger.Errorf("Failed to read user input: %v", err) -return +os.Exit(1) ```
Maintainability	Extract user confirmation logic into a separate function for better readability. ___ To improve code readability and maintainability, consider extracting the user confirmation logic into a separate function. This would make the main function cleaner and the logic reusable. [rmstale.go [77-87]](https://github.com/danstis/rmstale/pull/197/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR77-R87) ```diff -fmt.Printf("WARNING: Will remove files and folders recursively below '%v'%s older than %v days. Continue? (y/n) ", filepath.FromSlash(folder), extMsg, age) -var response string -_, err := fmt.Scanln(&response) -if err != nil { - logger.Errorf("Failed to read user input: %v", err) - return -} -if response != "y" && response != "Y" { +if !getUserConfirmation("WARNING: Will remove files and folders recursively below '%v'%s older than %v days. Continue? (y/n) ", filepath.FromSlash(folder), extMsg, age) { logger.Warning("Operation not confirmed, exiting.") return } ```

✨ Improve tool usage guide:

**Overview:** The `improve` tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on a PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L78) related to the improve tool (`pr_code_suggestions` section), use the following template: ``` /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_code_suggestions] some_config1=... some_config2=... ``` See the improve [usage page](https://pr-agent-docs.codium.ai/tools/improve/) for a comprehensive guide on using this tool.

sonarcloud[bot] commented 8 months ago

Quality Gate failed

Failed conditions
55.6% Coverage on New Code (required ≥ 60%)

See analysis details on SonarCloud

danstis / rmstale