hotfix CI deployment#13
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Hotfix to ensure the standalone GUI build artifacts include loc_config.yaml and to adjust CI artifact zipping/upload behavior.
Changes:
- Add
loc_config.yamlto PyInstaller--add-datafor Windows/macOS/Linux builds in CI and release workflows. - Remove
workflow_dispatch-only gating for zip/upload steps in CI (artifacts now upload on branch pushes as well).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/release.yml | Adds loc_config.yaml to packaged release assets across platforms (with a Windows separator issue to fix). |
| .github/workflows/ci.yml | Adds loc_config.yaml to CI builds and changes zip/upload steps to run on all configured push events (plus a Windows separator issue to fix). |
Comments suppressed due to low confidence (3)
.github/workflows/ci.yml:84
- These steps are labeled "manual runs only" but the
if: github.event_name == 'workflow_dispatch'guard was removed, so they will now run (and upload artifacts) on every push to the configured branches. Either restore theifcondition or update the step names/logic to reflect the new behavior to avoid confusion and unintended CI uploads.
- name: Zip Windows binary (manual runs only)
shell: pwsh
run: |
Move-Item -Force dist\VideoAnnotationTool.exe dist\VideoAnnotationTool-win.exe
Compress-Archive -Path dist\VideoAnnotationTool-win.exe -DestinationPath dist\VideoAnnotationTool-win.zip -Force
- name: Upload artifact (manual runs only)
uses: actions/upload-artifact@v4
with:
name: VideoAnnotationTool-Windows
path: annotation_tool/dist/VideoAnnotationTool-win.zip
retention-days: 3
.github/workflows/ci.yml:146
- These steps are labeled "manual runs only" but the
if: github.event_name == 'workflow_dispatch'guard was removed, so they will now run (and upload artifacts) on every push to the configured branches. Either restore theifcondition or update the step names/logic to reflect the new behavior to avoid confusion and unintended CI uploads.
- name: Zip macOS app (manual runs only)
shell: bash
run: |
ditto -c -k --sequesterRsrc --keepParent "dist/VideoAnnotationTool.app" "dist/VideoAnnotationTool-mac.zip"
- name: Upload artifact (manual runs only)
uses: actions/upload-artifact@v4
with:
name: VideoAnnotationTool-macOS
path: annotation_tool/dist/VideoAnnotationTool-mac.zip
retention-days: 3
.github/workflows/ci.yml:217
- These steps are labeled "manual runs only" but the
if: github.event_name == 'workflow_dispatch'guard was removed, so they will now run (and upload artifacts) on every push to the configured branches. Either restore theifcondition or update the step names/logic to reflect the new behavior to avoid confusion and unintended CI uploads.
- name: Zip Linux binary (manual runs only)
shell: bash
run: |
mv -f dist/VideoAnnotationTool dist/VideoAnnotationTool-linux
cd dist
zip -r VideoAnnotationTool-linux.zip VideoAnnotationTool-linux
cd ..
- name: Upload artifact (manual runs only)
uses: actions/upload-artifact@v4
with:
name: VideoAnnotationTool-Linux
path: annotation_tool/dist/VideoAnnotationTool-linux.zip
retention-days: 3
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --add-data "controllers;controllers" | ||
| --add-data "image;image" | ||
| --add-data "config.yaml;." | ||
| --add-data "loc_config.yaml:." |
There was a problem hiding this comment.
On Windows, PyInstaller expects --add-data paths to be separated with ; (as used in the other --add-data entries in this step). Using : here will treat the value as a single path and can break packaging. Please change this to use ; for the source/destination separator (e.g., keep it consistent with config.yaml;.).
Suggested change
| --add-data "loc_config.yaml:." | |
| --add-data "loc_config.yaml;." |
| --add-data "controllers;controllers" | ||
| --add-data "image;image" | ||
| --add-data "config.yaml;." | ||
| --add-data "loc_config.yaml:." |
There was a problem hiding this comment.
On Windows, PyInstaller expects --add-data paths to be separated with ; (as used in the other --add-data entries in this step). Using : here will treat the value as a single path and can break packaging. Please change this to use ; for the source/destination separator (e.g., keep it consistent with config.yaml;.).
Suggested change
| --add-data "loc_config.yaml:." | |
| --add-data "loc_config.yaml;." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.