Skip to content

Conversation

@o-l-a-v
Copy link
Contributor

@o-l-a-v o-l-a-v commented Oct 20, 2025

Fixed checkver using GitHub instead of SourceForge.

Changes:

  • Changed checkver and downloads to use GitHub.
  • Added 32bit and 64bit versions.
  • Use conventional PR title: <manifest-name[@version]|chore>: <general summary of the pull request>
  • I have read the Contributing Guide

Summary by CodeRabbit

Chores

  • Updated to version 4.8.33.236
  • Added dedicated 32-bit and 64-bit download packages
  • Improved update detection and delivery mechanism

Updated version and added architecture-specific URLs and hashes.
@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 20, 2025

Related: #16379.

@coderabbitai
Copy link

coderabbitai bot commented Oct 20, 2025

Walkthrough

The manifest for a software package is restructured to support architecture-specific downloads. A single URL/hash pair is split into separate 32bit and 64bit entries. Version is incremented to 4.8.33.236, checkver is switched from SourceForge to GitHub-based detection, and autoupdate is reconfigured accordingly.

Changes

Cohort / File(s) Summary
Architecture-specific manifest restructuring
bucket/mc.json
Reorganizes URL and hash fields into architecture object with 32bit and 64bit entries; updates version to 4.8.33.236; changes checkver from SourceForge-based regex to GitHub-based URL; restructures autoupdate to support per-architecture URLs

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • ScoopInstaller/Extras#16341: Modifies manifest checkver/autoupdate logic to handle architecture-specific download URLs, splitting 32bit/64bit handling with per-arch detection.

Suggested labels

review-needed

Suggested reviewers

  • z-Fng

Poem

🐰 A manifest tale, split in two—
32 and 64, both shiny and new!
From Forge to GitHub we now take flight,
Architecture-aware downloads, oh what a sight! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed The PR description is mostly complete and addresses the essential elements of the template. It includes a clear summary ("Fixed checkver using GitHub instead of SourceForge"), lists the main changes with bullet points, and confirms both required checklist items (conventional PR title and reading the Contributing Guide). The description aligns with the raw summary of changes. While the description lacks an explicit "Closes #" or "Relates to #" reference in the standard format, this information was provided by the author in the PR comments, and the description itself contains substantive information about the changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The pull request title "mc: Update to version 4.8.33.236, fix checkver & autoupdate" directly addresses the main changes in the changeset. The title mentions the specific version bump to 4.8.33.236, the checkver fix (changing from SourceForge to GitHub), and the autoupdate rework, which align with the key changes documented in the summary and PR objectives. The title is concise, clear, and specific enough that a teammate scanning the history would understand the primary modifications made to the mc.json file without being vague or misleading.
✨ 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.

@github-actions
Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

mc

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
bucket/mc.json (1)

30-30: Note: No hashes in autoupdate (defensible but worth awareness).

Following the pattern from compactgui.json (PR #16378), the autoupdate block lacks hashes because GitHub API doesn't reliably expose SHA-256 digests for release assets. This is an accepted trade-off in Scoop Extras. However, ensure that once the $buildVersion issue is resolved, hashes can be added if GitHub provides them.

Can you verify whether mcwin32 releases on GitHub provide SHA-256 checksums in release notes or as separate assets? If so, they could be extracted and added to the autoupdate block for enhanced security.

Also applies to: 33-33

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f20e280 and aa3d871.

📒 Files selected for processing (1)
  • bucket/mc.json (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: o-l-a-v
PR: ScoopInstaller/Extras#16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
Learnt from: Gitoffthelawn
PR: ScoopInstaller/Extras#16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds in recent versions uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format. This was confirmed by a working 10.0.0 update with valid hash.
📚 Learning: 2025-10-16T15:59:21.258Z
Learnt from: o-l-a-v
PR: ScoopInstaller/Extras#16349
File: bucket/fvim.json:45-49
Timestamp: 2025-10-16T15:59:21.258Z
Learning: In Scoop manifests, the `autoupdate.url` and `autoupdate.architecture.<arch>.url` fields must be valid URIs according to the JSON schema (defined with `"format": "uri"`). Variables like `$matchUrlx64` that contain only path segments must be combined with a base URL (e.g., `https://github.com/.../releases/download/`) to form a complete valid URI.

Applied to files:

  • bucket/mc.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: WindowsPowerShell
🔇 Additional comments (3)
bucket/mc.json (3)

2-15: Architecture and version structure looks good.

The migration to architecture-specific downloads with corresponding hashes is properly structured. Each entry points to the correct GitHub release assets.


24-26: GitHub checkver is appropriate.

The switch from SourceForge regex-based detection to GitHub is the right fix for issue #16379. This approach is more reliable for tracking mcwin32 releases.


27-36: The review comment is incorrect and should be disregarded.

$buildVersion is the 4th component of the manifest's version after splitting on dots. For example, if version = "3.7.1.2" then $majorVersion=3, $minorVersion=7, $patchVersion=1 and $buildVersion=2.

Since the manifest declares "version": "4.8.33.236", the $buildVersion variable automatically resolves to 236. The autoupdate URLs will correctly expand to:

  • 32bit: https://github.com/adamyg/mcwin32/releases/download/4.8.33.236/mcwin32-build236-setup.exe
  • 64bit: https://github.com/adamyg/mcwin32/releases/download/4.8.33.236/mcwin32-build236-x64-setup.exe

These match the hardcoded root-level URLs exactly. The code is correct as written.

Likely an incorrect or invalid review comment.

@z-Fng z-Fng changed the title [email protected]: Fixed checkver mc: Update to version 4.8.33.236, fix checkver & autoupdate Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant