Fix clean cache issue #3499
Fix clean cache issue #3499
Conversation
|
🚨 gitStream Monthly Automation Limit Reached 🚨 Your organization has exceeded the number of pull requests allowed for automation with gitStream. To continue automating your PR workflows and unlock additional features, please contact LinearB. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the cache clean-up issue by ensuring that plugin cache directories are deleted before the plugin directory is removed, preventing the settings page from freezing.
- Deletes plugin cache directories before deleting the overall plugin directory
- Adds a new helper method GetPluginCacheDir for clarity and separation of concerns
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Yusyuriv, onesounds Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe changes update the cache clearing logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPaneAboutViewModel
participant FileSystem
User->>SettingsPaneAboutViewModel: Trigger ClearCacheFolder()
SettingsPaneAboutViewModel->>FileSystem: Enumerate plugin cache subdirectories
loop For each plugin cache subdirectory
SettingsPaneAboutViewModel->>FileSystem: Delete subdirectory recursively (handle exceptions)
end
SettingsPaneAboutViewModel->>FileSystem: Delete plugin cache directory non-recursively (handle exceptions)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
This comment has been minimized.
This comment has been minimized.
| // Make sure directory clean | ||
| dir.Delete(true); | ||
| // Log folders are the last level of folders | ||
| dir.Delete(false); |
There was a problem hiding this comment.
can you show the parameter name so we know what option is set to false.
| try | ||
| { | ||
| // Log folders are the last level of folders | ||
| GetPluginCacheDir().Delete(false); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs (1)
235-261: Consider refactoring repeated try-catch blocks for directory deletion.There are multiple similar try-catch blocks for directory deletion throughout the code that could be refactored into a helper method.
You could create a helper method like:
private bool TryDeleteDirectory(DirectoryInfo directory, bool recursive) { try { directory.Delete(recursive); return true; } catch (Exception e) { App.API.LogException(ClassName, $"Failed to delete directory: {directory.Name}", e); return false; } }This would simplify your deletion logic and reduce code duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream workflow automation
- GitHub Check: build
🔇 Additional comments (4)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs (4)
189-190: Appropriate change to non-recursive deletion for log folders.The comment and change to
dir.Delete(false)makes sense since log folders are leaf nodes in the directory structure, so there's no need for recursive deletion.
218-218: Good addition of plugin cache directory reference.The introduction of a dedicated variable for the plugin cache directory improves readability and provides a clear separation between the main cache and plugin cache operations.
234-249: Good implementation of recursive deletion for plugin cache subdirectories.This change correctly implements the first step in the deletion process as mentioned in the PR description - deleting plugin subdirectories recursively. The comment explains the rationale clearly.
273-276: Good addition of the GetPluginCacheDir method.The new method follows the same pattern as other directory accessor methods and provides a clean way to access the plugin cache directory.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
Follow on with #3411.
Test