feat(update): add version check output to doctor and update#1836
feat(update): add version check output to doctor and update#1836reidliu41 wants to merge 1 commit into
Conversation
Add a check-only update path and surface version status in doctor. - Add `deepseek update --check` to report the latest release without downloading or replacing binaries - Show current/latest release status in `deepseek doctor` - Keep update installation behavior unchanged for `deepseek update` - Use async release checks in doctor so diagnostics do not panic inside the runtime
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a --check flag to the update command, allowing users to verify the latest release without performing an update. It also integrates a version check into the doctor command to display the current and latest versions. Feedback focuses on significant code duplication between the CLI and TUI crates for version parsing and comparison logic, suggesting these be moved to a shared utility module. Additionally, the reviewer noted redundant function calls and macro invocations in the doctor command implementation that should be refactored for better efficiency.
| fn doctor_release_base_url_from_env() -> Option<String> { | ||
| std::env::var(DOCTOR_RELEASE_BASE_URL_ENV) | ||
| .ok() | ||
| .or_else(|| std::env::var(DOCTOR_LEGACY_RELEASE_BASE_URL_ENV).ok()) | ||
| .map(|value| value.trim().to_string()) | ||
| .filter(|value| !value.is_empty()) | ||
| } | ||
|
|
||
| fn doctor_update_version_from_env() -> Option<String> { | ||
| std::env::var(DOCTOR_UPDATE_VERSION_ENV) | ||
| .ok() | ||
| .or_else(|| std::env::var(DOCTOR_LEGACY_UPDATE_VERSION_ENV).ok()) | ||
| .map(|value| value.trim().trim_start_matches('v').to_string()) | ||
| .filter(|value| !value.is_empty()) | ||
| } | ||
|
|
||
| fn compare_release_versions(current: &str, latest_tag: &str) -> std::cmp::Ordering { | ||
| parse_version_components(current).cmp(&parse_version_components(latest_tag)) | ||
| } | ||
|
|
||
| fn parse_version_components(version: &str) -> Vec<u64> { | ||
| let trimmed = version.trim().trim_start_matches('v'); | ||
| let core = trimmed.split(['-', '+', ' ']).next().unwrap_or(trimmed); | ||
| core.split('.') | ||
| .map(|part| { | ||
| part.chars() | ||
| .take_while(|ch| ch.is_ascii_digit()) | ||
| .collect::<String>() | ||
| .parse::<u64>() | ||
| .unwrap_or(0) | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
There's significant code duplication between crates/tui/src/main.rs and crates/cli/src/update.rs. The functions doctor_release_base_url_from_env, doctor_update_version_from_env, compare_release_versions, and parse_version_components are nearly identical to their counterparts in update.rs.
To improve maintainability and avoid having to update logic in multiple places, this shared functionality should be extracted into a common utility module or a new shared crate (e.g., deepseek-version-utils). This would also apply to the related constants and test cases.
| let icon = match compare_release_versions(env!("CARGO_PKG_VERSION"), &latest_tag) { | ||
| std::cmp::Ordering::Less => "!".truecolor(sky_r, sky_g, sky_b), | ||
| std::cmp::Ordering::Equal => "✓".truecolor(aqua_r, aqua_g, aqua_b), | ||
| std::cmp::Ordering::Greater => "·".dimmed(), | ||
| }; | ||
| println!(" {} latest: {}", icon, latest_tag); | ||
| match compare_release_versions(env!("CARGO_PKG_VERSION"), &latest_tag) { | ||
| std::cmp::Ordering::Less => { | ||
| println!(" Update available. Run `deepseek update` to install."); | ||
| } | ||
| std::cmp::Ordering::Equal => { | ||
| println!(" Already up to date."); | ||
| } | ||
| std::cmp::Ordering::Greater => { | ||
| println!(" Current build is newer than the latest published release."); | ||
| } | ||
| } |
There was a problem hiding this comment.
The compare_release_versions function is called twice with the same arguments. To improve efficiency and readability, you should call it once and store the result in a variable. You can also store env!("CARGO_PKG_VERSION") in a variable before line 1754 to avoid repeating the macro invocation.
|
Independent review: The Critical:
Recommendation: rebase on current main, drop everything outside v0.8.48 (PR #2256): 2 conflicts in |
|
This is opened with the old repo , so open a new branch to fix it in #2291, please help to check it. |
2 participants
Summary
Adds a lightweight way to check whether the installed binary is current.
Before this change,
deepseek updatecould perform an update, but there was no check-only path, anddeepseek doctordid not report whether a newer release was available. This made it harder for users to confirm whether local issues might already be fixed in a newer version.This PR adds:
deepseek update --checkto report the current version and latest release without downloading or replacing binariesUpdatessection indeepseek doctorv0.8.x,0.8.x, and build suffixes like0.8.x (commit)Testing
cargo test --all-featurescargo fmt --all -- --checkcargo clippy --all-targets --all-featuresChecklist