aegisub(-arch1t3cht): Persist automation folder#18007
Conversation
📝 WalkthroughWalkthroughAdds a post_install migration step to both bucket/aegisub.json and bucket/aegisub-arch1t3cht.json that, when an automation.original directory exists, copies its contents into automation and removes automation.original. Both manifests also add "automation" to their persist arrays so the automation directory is preserved across reinstalls/updates. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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.
🧹 Nitpick comments (1)
bucket/aegisub.json (1)
33-39: ⚡ Quick winConsider testing the persistence behavior locally.
To verify that the automation directory is correctly persisted across updates, you can test locally:
# Install the manifest scoop install .\bucket\aegisub.json # Add a test script to the automation directory New-Item "$env:USERPROFILE\scoop\apps\aegisub\current\automation\autoload\test-script.lua" -Force # Force reinstall to simulate an update scoop uninstall aegisub scoop install .\bucket\aegisub.json # Verify the test script still exists Test-Path "$env:USERPROFILE\scoop\apps\aegisub\current\automation\autoload\test-script.lua"If the test script survives the reinstall, the persistence is working correctly.
As per coding guidelines, you can also run the standard manifest validation:
# Auto-format (Lint) the JSON manifest .\bin\formatjson.ps1 -App aegisub🤖 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/aegisub.json` around lines 33 - 39, The manifest's "persist" array includes "automation", but you should verify persistence locally: install the manifest, place a test file under the app's automation\autoload directory, force-reinstall the app and confirm the test file still exists; also run the standard manifest validation/formatting (e.g., the formatjson.ps1 lint) to ensure JSON is well-formed—if the test file is lost, update the "persist" handling in aegisub.json (the "persist" array) to include the correct relative path or correct naming so the automation folder is persisted across reinstalls.Source: Coding guidelines
🤖 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/aegisub.json`:
- Around line 33-39: The manifest's "persist" array includes "automation", but
you should verify persistence locally: install the manifest, place a test file
under the app's automation\autoload directory, force-reinstall the app and
confirm the test file still exists; also run the standard manifest
validation/formatting (e.g., the formatjson.ps1 lint) to ensure JSON is
well-formed—if the test file is lost, update the "persist" handling in
aegisub.json (the "persist" array) to include the correct relative path or
correct naming so the automation folder is persisted across reinstalls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26367473-5304-4fcd-9597-466967399e0c
📒 Files selected for processing (1)
bucket/aegisub.json
z-Fng
changed the title
automation folder
z-Fng
left a comment
z-Fng
left a comment
There was a problem hiding this comment.
This change is incomplete and does not fully resolve the linking issue. There are a few remaining problems:
-
Expected/Current Behaviour: Note that this also happens for aegisub-arch1t3cht.
The
aegisub-arch1t3chtpackage also requires a fix. -
The
automationfolder is a default part of the package, not an empty directory. Persisting it directly causes the files within to remain stuck on the older version. Currently, the workaround is to manually overwrite the automation folder with the contents of the freshly downloadedautomationfolder after installation. While not ideal, it is still better than taking no action at all. Here's an example: #17092
arvdk
force-pushed
the
aegisub-persist-automation
branch
from
d8e40d7 to
2299c10
Compare
…3cht The automation folder has lots of files from installation. Now, a `scoop update` replaces those with the files from the update, leaving all other files untouched.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
bucket/aegisub.json (1)
27-34: Run the manifest checks locally before merge.Please validate both manifests with:
scoop config debug truescoop config gh_token <your-github-token>.\bin\checkver.ps1 -App aegisub -f.\bin\formatjson.ps1 -App aegisubscoop install .\bucket\aegisub.json -a 64bit.\bin\checkver.ps1 -App aegisub-arch1t3cht -f.\bin\formatjson.ps1 -App aegisub-arch1t3chtscoop install .\bucket\aegisub-arch1t3cht.json -a 64bitReferences:
- https://github.com/ScoopInstaller/.github/blob/main/.github/CONTRIBUTING.md
- https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests
As per coding guidelines, Scoop manifest reviews should include explicit local test/lint/checkver guidance and links to the contribution docs.
🤖 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/aegisub.json` around lines 27 - 34, Run the full local manifest validation and installation checks for the aegisub manifests and ensure the post_install script (the "post_install" array that runs the PowerShell ForEach-Object block) works as expected; specifically enable debug and set GH token then run: scoop config debug true, scoop config gh_token <your-github-token>, .\bin\checkver.ps1 -App aegisub -f, .\bin\formatjson.ps1 -App aegisub, scoop install .\bucket\aegisub.json -a 64bit, then repeat checkver/formatjson/install for aegisub-arch1t3cht (.\bin\checkver.ps1 -App aegisub-arch1t3cht -f and .\bin\formatjson.ps1 -App aegisub-arch1t3cht and scoop install .\bucket\aegisub-arch1t3cht.json -a 64bit); fix any lint/format/checkver failures and ensure the post_install Copy-Item/Remove-Item sequence behaves correctly locally before merging, following the Scoop CONTRIBUTING and App-Manifests guidance.Source: Coding guidelines
🤖 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/aegisub.json`:
- Around line 27-34: Run the full local manifest validation and installation
checks for the aegisub manifests and ensure the post_install script (the
"post_install" array that runs the PowerShell ForEach-Object block) works as
expected; specifically enable debug and set GH token then run: scoop config
debug true, scoop config gh_token <your-github-token>, .\bin\checkver.ps1 -App
aegisub -f, .\bin\formatjson.ps1 -App aegisub, scoop install
.\bucket\aegisub.json -a 64bit, then repeat checkver/formatjson/install for
aegisub-arch1t3cht (.\bin\checkver.ps1 -App aegisub-arch1t3cht -f and
.\bin\formatjson.ps1 -App aegisub-arch1t3cht and scoop install
.\bucket\aegisub-arch1t3cht.json -a 64bit); fix any lint/format/checkver
failures and ensure the post_install Copy-Item/Remove-Item sequence behaves
correctly locally before merging, following the Scoop CONTRIBUTING and
App-Manifests guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e175a3e7-2fe3-4ac9-99d2-e51ac542389d
📒 Files selected for processing (2)
bucket/aegisub-arch1t3cht.jsonbucket/aegisub.json
|
/verify |
|
All changes look good. Wait for review from human collaborators. aegisub-arch1t3cht
aegisub
|
2 participants
Closes #17957
<manifest-name[@version]|chore>: <general summary of the pull request>