gstreamer-np: Add 1.28.3#591
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds ChangesGStreamer Package Manifest
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
bucket/gstreamer.json (2)
6-7: 💤 Low valueConsider version-pinning the license URL.
The license URL points to the
masterbranch, which may change over time. For better stability and version accuracy, consider pinning to a specific version tag or commit.📋 Suggested improvement
- "identifier": "LGPL-2.1", - "url": "https://gitlab.freedesktop.org/gstreamer/gstreamer/-/raw/master/COPYING" + "identifier": "LGPL-2.1-or-later", + "url": "https://gitlab.freedesktop.org/gstreamer/gstreamer/-/raw/1.28.3/COPYING"🤖 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 `@bucket/gstreamer.json` around lines 6 - 7, The license "url" currently points at the mutable master branch; update the "url" value in gstreamer.json to a version-pinned URL (use a specific release tag or commit hash) so the license file is stable and reproducible — change the "url" string that pairs with "identifier": "LGPL-2.1" to reference a tagged release (or commit SHA) of the gstreamer repository instead of /raw/master/COPYING.
36-39: Checkver URL is functioning correctly.The page at https://gstreamer.freedesktop.org/download/ contains version numbers (1.28.3 confirmed) matching the regex pattern
(\d+?\.\d+?\.\d+?). The#windowsfragment does not affect the HTTP response. Consider simplifying the regex to(\d+\.\d+\.\d+)for better readability, as the non-greedy quantifiers are unnecessary for matching semantic version numbers.🤖 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 `@bucket/gstreamer.json` around lines 36 - 39, The checkver block contains an unnecessary non-greedy regex; update the "regex" value in the checkver object (the symbol named checkver with fields url and regex) to use a simpler, more readable pattern like (\d+\.\d+\.\d+) instead of (\d+?\.\d+?\.\d+?) while leaving the url (https://gstreamer.freedesktop.org/download/#windows) unchanged.
🤖 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 `@bucket/gstreamer.json`:
- Around line 31-34: The uninstaller redundantly elevates after already checking
is_admin; edit the script array entry that calls Invoke-ExternalCommand (the
string containing "Invoke-ExternalCommand \"$dir\\unins000.exe\" -ArgumentList
@('/VERYSILENT', '/SUPPRESSMSGBOXES', '/NORESTART') -RunAs") and remove the
-RunAs flag so it relies on the prior is_admin check (keep the admin-check line
using is_admin and the descriptive error string unchanged).
- Around line 24-28: The script redundantly elevates the installer: after
checking is_admin and breaking if false, remove the -Verb RunAs from the
Start-Process invocation so Start-Process("$dir\\install_gstreamer.exe"...
-ArgumentList ...) runs normally under the already-elevated script; keep the
is_admin check and the break as-is and leave the Remove-Item call unchanged.
---
Nitpick comments:
In `@bucket/gstreamer.json`:
- Around line 6-7: The license "url" currently points at the mutable master
branch; update the "url" value in gstreamer.json to a version-pinned URL (use a
specific release tag or commit hash) so the license file is stable and
reproducible — change the "url" string that pairs with "identifier": "LGPL-2.1"
to reference a tagged release (or commit SHA) of the gstreamer repository
instead of /raw/master/COPYING.
- Around line 36-39: The checkver block contains an unnecessary non-greedy
regex; update the "regex" value in the checkver object (the symbol named
checkver with fields url and regex) to use a simpler, more readable pattern like
(\d+\.\d+\.\d+) instead of (\d+?\.\d+?\.\d+?) while leaving the url
(https://gstreamer.freedesktop.org/download/#windows) unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb0f2f0e-9943-405c-9cc2-481f9d7fab3b
📒 Files selected for processing (1)
bucket/gstreamer.json
There was a problem hiding this comment.
🧹 Nitpick comments (3)
bucket/gstreamer-np.json (3)
33-33: 💤 Low valueConsider removing the pipe to
Out-Nullif unnecessary.
Invoke-ExternalCommandtypically handles output internally in Scoop. The pipe toOut-Nullmay be unnecessary, though this depends on Scoop's implementation. Review similar manifests in the repository to determine the standard pattern.🤖 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 `@bucket/gstreamer-np.json` at line 33, The manifest line invoking the uninstaller uses a trailing pipe to Out-Null which is likely unnecessary; remove the " | Out-Null" suffix from the Invoke-ExternalCommand call that runs "$dir\\unins000.exe" so the command relies on Scoop's Invoke-ExternalCommand output handling (match the pattern used by other manifests), then run or lint the manifest to confirm no behavioral change.
38-38: 💤 Low valueConsider simplifying the version regex to use greedy quantifiers.
The regex
(\d+?\.\d+?\.\d+?)uses non-greedy quantifiers, which work correctly but are less conventional for this pattern. Greedy quantifiers(\d+\.\d+\.\d+)are simpler and equally correct since the dot terminates digit matching.♻️ Simplified regex
-"regex": "(\\d+?\\.\\d+?\\.\\d+?)" +"regex": "(\\d+\\.\\d+\\.\\d+)"🤖 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 `@bucket/gstreamer-np.json` at line 38, The version-capturing regex in the "regex" field currently uses non-greedy quantifiers ("(\\d+?\\.\\d+?\\.\\d+?)"); update that value to use greedy quantifiers instead (e.g., "(\\d+\\.\\d+\\.\\d+)") so the pattern is simpler and conventional while preserving behavior—locate the "regex" field in gstreamer-np.json and replace the pattern string accordingly.
26-26: 💤 Low valueConsider removing the unnecessary pipe to
Out-Null.
Start-Processwith-Waitdoesn't produce pipeline output, so piping toOut-Nullhas no effect. The command will work correctly without it.♻️ Simplified version
-Start-Process "$dir\install_gstreamer.exe" -ArgumentList @('/SP-', '/VERYSILENT', '/SUPPRESSMSGBOXES', '/NORESTART', "/DIR=`"$dir`"") -Wait -Verb RunAs | Out-Null +Start-Process "$dir\install_gstreamer.exe" -ArgumentList @('/SP-', '/VERYSILENT', '/SUPPRESSMSGBOXES', '/NORESTART', "/DIR=`"$dir`"") -Wait -Verb RunAs🤖 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 `@bucket/gstreamer-np.json` at line 26, The command string containing Start-Process in gstreamer-np.json includes an unnecessary pipeline to Out-Null; remove the trailing " | Out-Null" from the Start-Process invocation (the string that begins with "Start-Process \"$dir\\install_gstreamer.exe\" -ArgumentList ... -Wait -Verb RunAs") so the command uses Start-Process with -Wait only and no piped Out-Null.
🤖 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.
Nitpick comments:
In `@bucket/gstreamer-np.json`:
- Line 33: The manifest line invoking the uninstaller uses a trailing pipe to
Out-Null which is likely unnecessary; remove the " | Out-Null" suffix from the
Invoke-ExternalCommand call that runs "$dir\\unins000.exe" so the command relies
on Scoop's Invoke-ExternalCommand output handling (match the pattern used by
other manifests), then run or lint the manifest to confirm no behavioral change.
- Line 38: The version-capturing regex in the "regex" field currently uses
non-greedy quantifiers ("(\\d+?\\.\\d+?\\.\\d+?)"); update that value to use
greedy quantifiers instead (e.g., "(\\d+\\.\\d+\\.\\d+)") so the pattern is
simpler and conventional while preserving behavior—locate the "regex" field in
gstreamer-np.json and replace the pattern string accordingly.
- Line 26: The command string containing Start-Process in gstreamer-np.json
includes an unnecessary pipeline to Out-Null; remove the trailing " | Out-Null"
from the Start-Process invocation (the string that begins with "Start-Process
\"$dir\\install_gstreamer.exe\" -ArgumentList ... -Wait -Verb RunAs") so the
command uses Start-Process with -Wait only and no piped Out-Null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a951612e-5a5d-41fe-b3a5-6da2aeb75428
📒 Files selected for processing (1)
bucket/gstreamer-np.json
|
/verify |
|
All changes look good. Wait for review from human collaborators. gstreamer-np
|
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with gstreamer-np
|
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with gstreamer-np
|
|
/verify |
|
All changes look good. Wait for review from human collaborators. gstreamer-np
|
1 participant
Closes #590