Add unobtrusive newer-version notice to check and environment commands#1262
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR adds a GitHub version-checking utility that detects newer dfetch releases and integrates update notifications into the ChangesGitHub Release Update Notification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.rst`:
- Line 4: The changelog entry only documents the new update notice for the
"dfetch check" command but omits the "dfetch environment" command; update the
CHANGELOG.rst entry to mention both commands and clarify their differing
behavior — state that "dfetch check" shows a dim notice below the header and is
skipped in CI environments, while the "dfetch environment" command (implemented
in dfetch.commands.environment) shows the notice unconditionally — keep the
wording concise and consistent with the existing bullet style.
In `@dfetch/commands/check.py`:
- Around line 102-108: The log interpolates raw GitHub tag text from
newer_version_available() into a Rich-formatted message (variable newer) which
could be interpreted as Rich markup; fix by escaping the value before
interpolation (use rich.markup.escape or equivalent) and log escape(newer) in
the logger.info call where newer is used so the message uses a safe, literal
version string.
In `@dfetch/commands/environment.py`:
- Around line 39-45: The Rich-formatted log interpolates the GitHub-derived
variable newer directly (see newer_version_available() and the logger.info
call), so escape the string before formatting to avoid accidental Rich markup
injection: import rich.markup.escape (or equivalent) and use escape(newer) in
the f-string passed to logger.info (e.g., f"[dim] dfetch {escape(newer)}
available — https://github.com/dfetch-org/dfetch/releases[/dim]"); no other code
paths need changing.
In `@tests/test_github_version_check.py`:
- Around line 23-71: Add three unit tests to tests/test_github_version_check.py
to cover the edge cases where the GitHub JSON response has (1) no "tag_name"
key, (2) an empty "tag_name" value (""), and (3) a non-string "tag_name" (e.g.
integer or null). For each new test (e.g. names like
test_newer_version_missing_tag_name_returns_none,
test_newer_version_empty_tag_name_returns_none,
test_newer_version_non_string_tag_name_returns_none) patch
dfetch.util.github_version_check.urllib.request.urlopen, return a mock response
whose read() yields the appropriate JSON bytes ({"some_other_key": "x"},
{"tag_name": ""}, {"tag_name": 123} or {"tag_name": null}), and assert that
newer_version_available() returns None; reuse the existing pattern of
context-manager mock setup used in other tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d945f83-d38c-40e5-8619-325c85fe572c
📒 Files selected for processing (5)
CHANGELOG.rstdfetch/commands/check.pydfetch/commands/environment.pydfetch/util/github_version_check.pytests/test_github_version_check.py
Poll the GitHub releases API (api.github.com/repos/dfetch-org/dfetch/releases/latest) and display a dim hint when a newer dfetch release is available. - dfetch/util/github_version_check.py: pure utility — fetches latest release tag and returns it if newer than the running version, silently swallowing any network/parse failure - dfetch/commands/check.py: calls it when CI env var is not set - dfetch/commands/environment.py: calls it unconditionally (diagnostic command) - api.github.com:443 is already in the harden-runner allowlist in test.yml https://claude.ai/code/session_011XLxZxgRCWhSc5PfHGam5P
…line Removes `BytesIO` and `pytest` from test_github_version_check.py (both unused, flagged by ruff F401). Black also reformats the _RELEASES_URL constant to a single line since it fits within the 88-char limit. https://claude.ai/code/session_011XLxZxgRCWhSc5PfHGam5P
spoorcc
force-pushed
the
claude/dfetch-version-check-8bkjb0
branch
from
b21c5f0 to
dd5c6ec
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
CHANGELOG.rst (1)
4-4: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winClarify the release-note wording.
This still reads like a generic “check for new version” note; it doesn’t reflect the actual user-visible split between
dfetch check(skips CI) anddfetch environment(always shows the notice). Updating the bullet to match that behavior would make the changelog more accurate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.rst` at line 4, The changelog entry for issue `#1262` in CHANGELOG.rst line 4 is too generic and doesn't accurately reflect the different behavior between the two commands. Reword the bullet point to clarify that dfetch check skips the version notice during CI environments, while dfetch environment consistently displays the notice regardless of the CI context. This will make the release note more precise and informative about the actual user-visible behavior differences.dfetch/util/github_version_check.py (1)
41-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explanatory comment for intentional exception swallowing.
The empty
exceptclause lacks a comment explaining that failures are intentionally ignored for best-effort update checks. This violates the "avoid suppressions without fixing root cause" guideline (though the barepassis intentional here, documentation is still required).📝 Proposed fix
except (urllib.error.URLError, json.JSONDecodeError, ValueError): + # Best-effort check: silently ignore network/parse failures so CLI remains functional pass return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dfetch/util/github_version_check.py` around lines 41 - 42, The except clause in the version check function that catches URLError, JSONDecodeError, and ValueError lacks a comment explaining the intentional exception suppression. Add a comment above or within the except block (before the pass statement) that clearly documents that these exceptions are intentionally being ignored because the version check is a best-effort operation and failures should not impact the main application functionality.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@CHANGELOG.rst`:
- Line 4: The changelog entry for issue `#1262` in CHANGELOG.rst line 4 is too
generic and doesn't accurately reflect the different behavior between the two
commands. Reword the bullet point to clarify that dfetch check skips the version
notice during CI environments, while dfetch environment consistently displays
the notice regardless of the CI context. This will make the release note more
precise and informative about the actual user-visible behavior differences.
In `@dfetch/util/github_version_check.py`:
- Around line 41-42: The except clause in the version check function that
catches URLError, JSONDecodeError, and ValueError lacks a comment explaining the
intentional exception suppression. Add a comment above or within the except
block (before the pass statement) that clearly documents that these exceptions
are intentionally being ignored because the version check is a best-effort
operation and failures should not impact the main application functionality.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f32ac126-abcd-4b2e-868e-e88e43ffa96d
📒 Files selected for processing (9)
CHANGELOG.rstdfetch/commands/check.pydfetch/commands/environment.pydfetch/log.pydfetch/util/github_version_check.pyfeatures/environment.featurefeatures/environment.pyfeatures/steps/generic_steps.pytests/test_github_version_check.py
spoorcc
force-pushed
the
claude/dfetch-version-check-8bkjb0
branch
from
dd5c6ec to
4636dde
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
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
2 participants
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.
Poll the GitHub releases API (api.github.com/repos/dfetch-org/dfetch/releases/latest)
and display a dim hint when a newer dfetch release is available.
tag and returns it if newer than the running version, silently swallowing
any network/parse failure
https://claude.ai/code/session_011XLxZxgRCWhSc5PfHGam5P
Summary by CodeRabbit
New Features
dfetch check, a dim notice now appears under the header if a newer GitHub release is available (skipped in CI).dfetch environmentnow shows the installeddfetchversion and, when applicable, adds a “newer version available” notice with a releases link.Tests
dfetch environment.