-
-
Notifications
You must be signed in to change notification settings - Fork 414
Refactor CheckForUpdates #797
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
Conversation
Split into logically named helper functions: - readLastCheckTime - shouldCheckForUpdate - checkAndNotifyUpdate - fetchLatestRelease - notifyUpdateAvailable
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Main
participant FileSystem
participant GitHubAPI
User->>Main: Start application
Main->>FileSystem: readLastCheckTime()
FileSystem-->>Main: lastCheckTime
Main->>Main: shouldCheckForUpdate(now, lastCheckTime)
alt Update Needed
Main->>GitHubAPI: fetchLatestRelease()
GitHubAPI-->>Main: latestReleaseInfo
Main->>Main: checkAndNotifyUpdate(currentTime)
alt Newer Version Available
Main->>User: notifyUpdateAvailable(latest)
end
end
Main->>FileSystem: Write current time as last check
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cmd/main.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lazysegtree
PR: yorukot/superfile#0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR #767 for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.
🪛 GitHub Check: test
src/cmd/main.go
[failure] 310-310:
undefined: variable.LastVersionURL
[failure] 305-305:
undefined: release
[failure] 304-304:
undefined: release
[failure] 298-298:
declared and not used: body
[failure] 288-288:
undefined: context.withTimeout
[failure] 277-277:
undefined: timeTime
[failure] 274-274:
undefined: timeParse
[failure] 270-270:
undefined: nill
[failure] 253-253:
undefined: commonConfig
🪛 GitHub Check: build
src/cmd/main.go
[failure] 310-310:
undefined: variable.LastVersionURL
[failure] 305-305:
undefined: release
[failure] 304-304:
undefined: release
[failure] 298-298:
declared and not used: body
[failure] 288-288:
undefined: context.withTimeout
[failure] 277-277:
undefined: timeTime
[failure] 274-274:
undefined: timeParse
[failure] 270-270:
undefined: nill
[failure] 253-253:
undefined: commonConfig
🪛 GitHub Actions: Go
src/cmd/main.go
[error] 6-6: "encoding/json" imported and not used
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: 2
♻️ Duplicate comments (1)
src/cmd/main.go (1)
252-253:⚠️ Potential issueFix the reference to the configuration object to avoid build failure.
The code is referencing
commonConfigwhich is undefined, causing build and test failures. You should use the already importedcommon.Configobject.func CheckForUpdates(){ - if !commonConfig.AutoCheckUpdate{ + if !common.Config.AutoCheckUpdate{🧰 Tools
🪛 GitHub Check: build
[failure] 253-253:
undefined: commonConfig🪛 GitHub Check: test
[failure] 253-253:
undefined: commonConfig🪛 GitHub Actions: Go
[error] 253-253: Build failed: undefined identifier 'commonConfig'
🪛 GitHub Actions: Python Testsuite Run
[error] 253-253: undefined: commonConfig
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cmd/main.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lazysegtree
PR: yorukot/superfile#0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR #767 for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.
🪛 GitHub Check: build
src/cmd/main.go
[failure] 304-304:
undefined: GitHubRelease
[failure] 253-253:
undefined: commonConfig
🪛 GitHub Check: test
src/cmd/main.go
[failure] 304-304:
undefined: GitHubRelease
[failure] 253-253:
undefined: commonConfig
🪛 GitHub Actions: Go
src/cmd/main.go
[error] 253-253: Build failed: undefined identifier 'commonConfig'
🪛 GitHub Actions: Python Testsuite Run
src/cmd/main.go
[error] 253-253: undefined: commonConfig
🔇 Additional comments (4)
src/cmd/main.go (4)
268-281: ThereadLastCheckTimeimplementation looks good.This helper function correctly handles error cases and properly formats time values, improving the readability of the main function.
283-285: Clean implementation of update check logic.This function clearly encapsulates the decision logic for when to perform the update check.
315-321: ThefetchLatestReleaseimplementation is clean and focused.This function properly handles HTTP requests with context and has good error handling.
323-335: Nice formatting of the notification message.The notification message is well-formatted using lipgloss styles, which improves user experience.
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: 2
🧹 Nitpick comments (3)
src/cmd/main.go (3)
304-306: Consider moving theGitHubReleasetype definition outside the function.For better code organization and reusability, consider moving this type definition to the package level or a separate models package:
+// GitHubRelease represents the GitHub API response for release information +type GitHubRelease struct { + TagName string `json:"tag_name"` +} + func checkAndNotifyUpdate() { // ... existing code ... - type GitHubRelease struct { - TagName string `json:"tag_name"` - }This would allow other functions to reuse this type if needed in the future.
327-339: Well-formatted notification with clear visual styling.The notification implementation is visually appealing and clearly communicates the update availability to users.
Consider adding a conditional check for silent mode or quiet mode (if your application supports it) to avoid printing notifications in non-interactive contexts.
249-339: Overall excellent refactoring that improves code maintainability.The refactoring of the
CheckForUpdatesfunctionality has successfully broken down a monolithic function into smaller, focused helper functions. Each function has a clear responsibility, making the code easier to understand and maintain.Benefits of this refactoring:
- Improved readability through smaller, focused functions
- Better error handling with appropriate logging
- Clear separation of concerns
- Easier testing of individual components
- More descriptive function names that document their purpose
Please fix the formatting issues by running
go fmt ./...to resolve the pipeline failures.🧰 Tools
🪛 golangci-lint (1.64.8)
252-252: File is not properly formatted
(goimports)
🪛 GitHub Actions: Go
[error] 249-281: go fmt check failed: code formatting issues detected. Please run 'go fmt ./...' to fix formatting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cmd/main.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lazysegtree
PR: yorukot/superfile#0
File: :0-0
Timestamp: 2025-04-12T12:00:32.688Z
Learning: In PR #767 for yorukot/superfile, the focus is on moving code (especially sidebar-related functionality) to a more organized structure without changing functionality. Pre-existing issues should be ignored since the purpose is code reorganization, not fixing existing problems.
🪛 golangci-lint (1.64.8)
src/cmd/main.go
252-252: File is not properly formatted
(goimports)
🪛 GitHub Actions: Go
src/cmd/main.go
[error] 249-281: go fmt check failed: code formatting issues detected. Please run 'go fmt ./...' to fix formatting.
🔇 Additional comments (3)
src/cmd/main.go (3)
283-285: Good implementation of the update check condition.This short, focused helper function clearly expresses the condition for when an update check should occur. It's a good example of the single responsibility principle.
287-317: Well-structured update check with proper error handling.The function effectively:
- Creates a timeout context (good practice for HTTP requests)
- Uses dedicated helper functions for different concerns
- Has proper error handling with helpful log messages
- Correctly defines and parses the GitHub API response
This is a significant improvement over a monolithic implementation.
319-325: Good separation of HTTP request logic.Extracting the HTTP request logic into a separate function improves testability and makes the code more modular. It also follows the single responsibility principle.
|
This looks good. @JassonCordones Thanks for this. |
|
Also please squash all the comments for |
|
Hi, Thanks for Merging. I haven't been able to work on here these last few weeks |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [yorukot/superfile](https://github.com/yorukot/superfile) | minor | `v1.2.1` -> `v1.3.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>yorukot/superfile (yorukot/superfile)</summary> ### [`v1.3.0`](https://github.com/yorukot/superfile/releases/tag/v1.3.0) [Compare Source](yorukot/superfile@v1.2.1...v1.3.0) We are excited to release v1.3.0 for superfile, with new features, key bug fixes, and lots of polish under the hood. #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - We have revamped the command prompt and added built-in commands for SuperFile-specific actions. Check out yorukot/superfile#745 - Multi-panel startup support. You can now open multiple panels right from startup, making your workflow even more efficient. - Added new configurations : --chooser-file option, show_panel_footer_info config flag and many command prompt specific flags. #### Improvements & Fixes - The sidebar code was refactored and separated for better maintainability and various linter fixes and CI/CD improvements were made to keep the codebase clean and robust. - A new Rendering package is implemented, centralising border, content, and section rendering logic into reusable renderer components, fixing many layout bugs. - Model behaviour, file operations and rendering-related unit tests were added to improve test coverage. #### Detailed Change Summary <details><summary>Details</summary> <p> #### New Features - Added a Command-Prompt for SuperFile specific actions [`#752`](yorukot/superfile#752) by [@​Rocco-Gossmann](https://github.com/Rocco-Gossmann), [@​yorukot](https://github.com/yorukot) and [@​lazysegtree](https://github.com/lazysegtree) - Allow specifying multiple panels at startup [`#759`](yorukot/superfile#759) by [@​lazysegtree](https://github.com/lazysegtree) - Initial draft of rendering package [`#775`](yorukot/superfile#775) by [@​lazysegtree](https://github.com/lazysegtree) - Render unit tests for prompt model [`#809`](yorukot/superfile#809) by [@​lazysegtree](https://github.com/lazysegtree) - Chooser file option, --lastdir-file option, and improvements in quit, and bug fixes [`#812`](yorukot/superfile#812) by [@​lazysegtree](https://github.com/lazysegtree) - Prompt feature leftover items [`#804`](yorukot/superfile#804) by [@​lazysegtree](https://github.com/lazysegtree) - SPF Prompt tutorial and fixes [`#814`](yorukot/superfile#814) by [@​lazysegtree](https://github.com/lazysegtree) - Write prompt tutorial, rename prompt mode to spf mode, add develop branch in GitHub workflow, show_panel_footer_info flag [`#815`](yorukot/superfile#815) by [@​lazysegtree](https://github.com/lazysegtree) - Theme: Add gruvbox-dark-hard [`#828`](yorukot/superfile#828) by [@​Frost-Phoenix](https://github.com/Frost-Phoenix) #### Updates & Improvements - Sidebar separation [`#767`](yorukot/superfile#767) by [@​lazysegtree](https://github.com/lazysegtree) - Sidebar code separation [`#770`](yorukot/superfile#770) by [@​lazysegtree](https://github.com/lazysegtree) - Rendering package and rendering bug fixes [`#781`](yorukot/superfile#781) by [@​lazysegtree](https://github.com/lazysegtree) - Refactor CheckForUpdates [`#797`](yorukot/superfile#797) by [@​JassonCordones](https://github.com/JassonCordones) - Rename metadata strings [`#731`](yorukot/superfile#731) by [@​booth-w](https://github.com/booth-w) #### Bug Fixes - Fix crash with opening file with editor on an empty panel [`#730`](yorukot/superfile#730) by [@​booth-w](https://github.com/booth-w) - Fix: Add some of the remaining linter and fix errors [`#756`](yorukot/superfile#756) by [@​lazysegtree](https://github.com/lazysegtree) - Golangci lint fixes [`#757`](yorukot/superfile#757) by [@​lazysegtree](https://github.com/lazysegtree) - Fix: Remove redundant function containsKey [`#765`](yorukot/superfile#765) by [@​lazysegtree](https://github.com/lazysegtree) - Fix: Correctly resolve path in open and cd prompt actions [`#802`](yorukot/superfile#802) by [@​lazysegtree](https://github.com/lazysegtree) - Prompt dynamic dimensions and unit tests fix [`#805`](yorukot/superfile#805) by [@​lazysegtree](https://github.com/lazysegtree) - Fix: Convert unicode space to normal space, use rendered in file preview to fix layout bugs, Release 1.3.0 [`#825`](yorukot/superfile#825) by [@​lazysegtree](https://github.com/lazysegtree) #### Optimization & Code Quality - Adding linter to CI/CD and fix some lint issues [`#739`](yorukot/superfile#739) by [@​lazysegtree](https://github.com/lazysegtree) - Linter fixes, new feature of allowing multiple directories at startup, other code improvements [`#764`](yorukot/superfile#764) by [@​lazysegtree](https://github.com/lazysegtree) - Model unit tests [`#803`](yorukot/superfile#803) by [@​lazysegtree](https://github.com/lazysegtree) #### Dependency Updates - fix(deps): update dependency astro to v5.7.7 [`#726`](yorukot/superfile#726) by [@​renovate](https://github.com/renovate) - fix(deps): update module github.com/shirou/gopsutil/v4 to v4.25.3 [`#749`](yorukot/superfile#749) by [@​renovate](https://github.com/renovate) - fix(deps): update module github.com/pelletier/go-toml/v2 to v2.2.4 [`#760`](yorukot/superfile#760) by [@​renovate](https://github.com/renovate) - fix(deps): update module github.com/alecthomas/chroma/v2 to v2.16.0 [`#751`](yorukot/superfile#751) by [@​renovate](https://github.com/renovate) - fix(deps): update dependency sharp to ^0.34.0 [`#755`](yorukot/superfile#755) by [@​renovate](https://github.com/renovate) - fix(deps): update dependency [@​astrojs/starlight](https://github.com/astrojs/starlight) to ^0.34.0 [`#761`](yorukot/superfile#761) by [@​renovate](https://github.com/renovate) </p> </details> #### New Contributors * @​Rocco-Gossmann made their first contribution in yorukot/superfile#736 * @​Frost-Phoenix made their first contribution in yorukot/superfile#828 **Full Changelog**: yorukot/superfile@v1.2.1...v1.3.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC4yMi4wIiwidXBkYXRlZEluVmVyIjoiNDAuMjMuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Split into logically named helper functions:
Summary by CodeRabbit