Skip to content

fix(checkver): Remove redundant always-true condition in GitHub checkver logic#6571

Merged
niheaven merged 2 commits into
ScoopInstaller:developfrom
z-Fng:remove-always-true-condition
Dec 30, 2025
Merged

fix(checkver): Remove redundant always-true condition in GitHub checkver logic#6571
niheaven merged 2 commits into
ScoopInstaller:developfrom
z-Fng:remove-always-true-condition

Conversation

@z-Fng

@z-Fng z-Fng commented Dec 18, 2025

Copy link
Copy Markdown
Member

Motivation and Context

Description

Scoop/bin/checkver.ps1

Lines 159 to 163 in b588a06

if ($json.checkver.github) {
$url = $json.checkver.github.TrimEnd('/') + '/releases/latest'
$regex = $githubRegex
if ($json.checkver.PSObject.Properties.Count -eq 1) { $useGithubAPI = $true }
}

The previous property count check was always true and had no effect. The always-true condition was misleading and confusing.
This change removes the redundant condition and explicitly enables the GitHub API for GitHub-based checkver.

$content = '{"checkver":{"github":"https://github.com/ScoopInstaller/Scoop","regex":"[\\d.]+"}}'
$json = ConvertFrom-Json $content
if ($json.checkver.PSObject.Properties.Count -eq 1) {Write-Host 'Condition passed'}
$json.checkver.PSObject.Properties.Count
$json.checkver | Get-Member -MemberType NoteProperty | Measure-Object | Select-Object -ExpandProperty Count
image

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

Summary by CodeRabbit

  • Bug Fixes

    • GitHub-based version checks now consistently use the GitHub API, removing redundant conditional behavior for more reliable lookups.
  • Documentation

    • Added an unreleased changelog entry noting the redundant-condition removal in the GitHub checkver flow.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 18, 2025

Copy link
Copy Markdown

Walkthrough

The PR removes a redundant condition in GitHub checkver logic so that GitHub API usage is always enabled for manifest entries using checkver.github; the change affects bin/checkver.ps1 and adds a changelog entry.

Changes

Cohort / File(s) Summary
Documentation
CHANGELOG.md
Adds an Unreleased entry documenting the removal of an always-true condition in the GitHub checkver logic.
GitHub checkver logic
bin/checkver.ps1
Removes the PSObject.Properties.Count conditional; useGithubAPI is now set to true whenever a manifest entry uses checkver.github, always routing through the GitHub API path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect bin/checkver.ps1 to confirm the removed condition was redundant and verify no other code relies on the previous branching.
  • Check URL transformation and authorization code paths that execute when useGithubAPI is true for unintended side effects.
  • Run or simulate checkver flows with different manifest shapes to ensure no regressions.

Suggested reviewers

  • niheaven

Poem

🐰 I hopped through the branches, found a clause in the grass,
Always true and quite needless — I gave it a pass.
Now paths to GitHub hum clearly by night,
Simpler and cleaner, the logic feels light. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main change: removing a redundant always-true condition in the GitHub checkver logic, which matches the modifications in bin/checkver.ps1 and the CHANGELOG entry.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@z-Fng

z-Fng commented Dec 19, 2025

Copy link
Copy Markdown
Member Author

Perhaps we could avoid creating a separate changelog entry for this PR and instead use the same one as #6547?

checkver: Fix incorrect version returned when script fails without output (#6547)

@niheaven

Copy link
Copy Markdown
Member

A separate entry is acceptable.

@niheaven niheaven merged commit bdf6344 into ScoopInstaller:develop Dec 30, 2025
3 checks passed
@z-Fng z-Fng deleted the remove-always-true-condition branch December 30, 2025 03:54
Deide pushed a commit to Deide/scoop that referenced this pull request Feb 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants