Merged
Conversation
- Remove silent auto-update mechanism, replace with update check that notifies users when a new version is available - Add `deepsource update` subcommand for explicit manual updates - Remove AutoUpdate config option since updates are now user-initiated - Rename ShouldAutoUpdate to ShouldCheckForUpdate to reflect new behavior
- Replace style.Successf with fmt.Fprintf for the "already up to date" message - Keeps output simple and consistent with non-styled update flow
- Skip update check when running update command - Colorize update notification - Show examples only with --help -v - Simplify update success message
- Derive binary name from os.Args[0] instead of hardcoding 'deepsource' - Replace em dash with comma in the message
|
|
Overall Grade |
Security Reliability Complexity Hygiene Coverage |
Feedback
- Centralize validation for all external update artifacts
- Integrity checks, path-sanitization, and metadata parsing all guard the same boundary: untrusted update data. Consolidate those checks into a single validation/ingestion layer so every update path reuses the same policy and avoids gaps.
- Make update lifecycle explicit, not implicit
- Automatic background timers, one-off notifications, and removed config flags are symptoms of mixed implicit behaviors. Define clear start/stop/check APIs and use callers to drive state so behavior is discoverable and testable rather than buried in implicit tasks.
- One coordinator owns update state and concurrency
- Multiple guards against "redundant checks" and ad-hoc in-progress flags indicate scattered state handling. Replace scattered flags with a single update coordinator (with an in-progress lock and clear state transitions) to prevent races and duplicated logic.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Go | Mar 20, 2026 11:59p.m. | Review ↗ | |
| Secrets | Mar 20, 2026 11:59p.m. | Review ↗ | |
| Test coverage | Mar 20, 2026 11:59p.m. | Review ↗ |
Code Coverage Summary
| Language | Line Coverage (New Code) | Line Coverage (Overall) |
|---|---|---|
| Aggregate | 10% [⤫ below threshold] |
24.8% [▼ down 0.1% from master] |
| Go | 10% [⤫ below threshold] |
24.8% [▼ down 0.1% from master][✓ above threshold] |
➟ Additional coverage metrics may have been reported. See full coverage report ↗
sourya-deepsource
approved these changes
Mar 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.