feat: add a whatif/dry run option

danstis commented 7 months ago

User description

Fixes #186

Type

enhancement, tests

Description

Introduced a dry run mode to the application, allowing users to simulate file removal without actually deleting any files. This is useful for verifying what would be deleted in a real run.
Added a dryRun boolean flag that can be set via -d or --dry-run command-line options.
Modified the procDir and removeItem functions to support dry run mode by skipping the actual file removal process if dryRun is true.
Expanded the test suite to include tests for the new dry run functionality, ensuring that no files are removed when the mode is active.

Changes walkthrough

Relevant files

Enhancement

rmstale.go

Implement Dry Run Mode for Safe Operation

rmstale.go

Added a dryRun boolean flag to enable dry run mode where files are not
actually removed.

Modified procDir and removeItem functions to accept dryRun as a
parameter and conditionally skip file removal.

Updated flag parsing to include the dryRun option with -d and
--dry-run flags.

+13/-6

Tests

rmstale_test.go

Extend Tests to Cover Dry Run Mode

rmstale_test.go

Extended test cases to include the dryRun parameter.

Added a new test suite TestDryRunOption to verify that files are not
removed when dryRun is true.

+40/-4

✨ 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 7 months ago

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

sophie-syntax[bot] commented 7 months ago

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and well-documented, focusing on implementing a dry run feature and updating tests accordingly. The logic is simple and follows the existing code patterns.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Issue: The implementation assumes that logging the dry run action is sufficient for all use cases. In some scenarios, users might expect a more detailed report or a different format of the output when using the dry run option.
🔍 Possible issues	Consistency: The `dryRun` parameter is added to the `procDir` and `removeItem` functions. It's important to ensure all calls to these functions across the project are updated accordingly to avoid compilation errors or unexpected behavior.
🔒 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 7 months ago

PR Code Suggestions

Category	Suggestions
Enhancement	Simplify the command-line interface by using a single flag for the dry run option. ___ Instead of defining two flags for the dry run option (`-d` and `--dry-run`), consider using a single flag to simplify the command-line interface. This approach reduces confusion and ensures consistency in how users interact with the application. [rmstale.go [51-52]](https://github.com/danstis/rmstale/pull/201/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR51-R52) ```diff -flag.BoolVar(&dryRun, "d", false, "Dry run mode, no files will be removed.") flag.BoolVar(&dryRun, "dry-run", false, "Dry run mode, no files will be removed.") ```
Enhancement	Make dry run logs clearly distinguishable by prefixing them with `[DRY RUN]`. ___ When implementing the dry run functionality, it's crucial to ensure that the application's output clearly distinguishes between actions that would be taken versus actions that are actually taken. Consider prefixing dry run logs with `[DRY RUN]` to make it unmistakably clear to the user. [rmstale.go [187]](https://github.com/danstis/rmstale/pull/201/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR187-R187) ```diff -logger.Infof("Dry run: '%v' would be removed...", filepath.FromSlash(fp)) +logger.Infof("[DRY RUN] '%v' would be removed...", filepath.FromSlash(fp)) ```
Maintainability	Improve code readability by extracting flag registration into a separate function. ___ To improve code readability and maintainability, consider extracting the flag registration (e.g., for the dry run option) into a separate function. This approach helps keep the `main` function concise and focused on high-level logic. [rmstale.go [52]](https://github.com/danstis/rmstale/pull/201/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR52-R52) ```diff -flag.BoolVar(&dryRun, "dry-run", false, "Dry run mode, no files will be removed.") +registerFlags() +... +func registerFlags() { + flag.BoolVar(&dryRun, "dry-run", false, "Dry run mode, no files will be removed.") + ... +} ```
Maintainability	Add a detailed comment for the `dryRun` flag to clarify its purpose. ___ To ensure that the `dryRun` flag's purpose is immediately clear to future maintainers or users of the code, consider adding a more detailed comment explaining the flag's effect on the program's behavior, especially in the context of file deletion. [rmstale.go [52]](https://github.com/danstis/rmstale/pull/201/files#diff-8d2045f56d565d537deafa629d12ea2b52f0701a7366f155b4b1038745e58e4eR52-R52) ```diff +// The dryRun flag, when set to true, enables the application to simulate the deletion of files without actually removing them. This is useful for testing or verifying what would be deleted in a real run. flag.BoolVar(&dryRun, "dry-run", false, "Dry run mode, no files will be removed.") ```

✨ 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.