-
Notifications
You must be signed in to change notification settings - Fork 1.5k
traindir3:Add version 39 #16423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
traindir3:Add version 39 #16423
Conversation
WalkthroughAdds a new JSON package manifest for Train Director 3 at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 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 |
/verify |
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with traindir3
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with traindir3
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/traindir3.json
(1 hunks)
🔇 Additional comments (2)
bucket/traindir3.json (2)
6-7
: Verify download URL and hash validity.Ensure the download URL is still active and the SHA256 hash matches the actual downloaded file. This should ideally be tested before merging.
15-21
: $cleanVersion is valid; regex matching requires manual verification.Based on Scoop documentation, $cleanVersion is a documented version variable available for autoupdate URL patterns. Your autoupdate URL structure is correct.
However, the regex pattern
"Train Director Version ([\\d.]+)"
cannot be verified without accessing the actual webpage athttps://www.backerstreet.com/traindir/en/download.php
. You should manually check that this regex correctly captures version strings from that page before merging.
/verify |
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with traindir3
|
/verify |
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with traindir3
|
/verify |
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with traindir3
|
/verify |
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with traindir3
|
ToT |
ToT |
/verify |
All changes look good. Wait for review from human collaborators. traindir3
|
ToT |
/verify |
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with traindir3
|
There was a problem hiding this 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/traindir3.json (1)
15-21
: Consider a more conventional checkver/autoupdate pattern.The checkver regex
(tdir[0-9]+win\.zip)
captures the full filename pattern rather than extracting a semantic version number. While this works for the autoupdate URL pattern (since$version
resolves totdir39win.zip
), it's unconventional for Scoop and less maintainable. A more robust approach would extract just the version number (e.g.,39
→3.9
) and construct the filename in autoupdate.Consider refactoring to extract semantic version:
"checkver": { "url": "https://www.backerstreet.com/traindir/en/trdireng.php", - "regex": "(tdir[0-9]+win\\.zip)" + "regex": "tdir([0-9])([0-9])win\\.zip", + "replace": "$1.$2" }, "autoupdate": { - "url": "https://www.backerstreet.com/traindir/$version" + "url": "https://www.backerstreet.com/traindir/tdir${version/\./}win.zip" }This extracts the core version numbers and reconstructs the filename explicitly, making the version detection more maintainable and aligned with typical Scoop patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/traindir3.json
(1 hunks)
🔇 Additional comments (2)
bucket/traindir3.json (2)
7-7
: Verify the SHA256 hash matches the actual download.Confirm that the hash
87384ce99ed3bf5b34dc9d3fcdbbcb2cc2bcec73298bd61ac087ecce22d1f365
is valid and corresponds to the file athttps://www.backerstreet.com/traindir/tdir39win.zip
.
5-5
: License field is correct for Train Director 3.Train Director 3 is GPL/freeware distributed by BackerStreet, confirming that "GPL-2.0-only" is the appropriate license value. The previous concern about the license value changing is resolved—this is the correct license for this project. The leading whitespace issue previously flagged is also already fixed.
However, two secondary concerns from the original review remain:
The
checkver
regex(tdir[0-9]+win\.zip)
captures the full filename pattern instead of extracting a semantic version number (e.g., "3.9"). This is non-standard for Scoop manifests but functionally works.The SHA256 hash should be verified against the actual download from BackerStreet to ensure integrity.
/verify |
All changes look good. Wait for review from human collaborators. traindir3
|
ok i fix it |
i`m ready |
<manifest-name[@version]|chore>: <general summary of the pull request>
Summary by CodeRabbit
Summary by CodeRabbit