-
Notifications
You must be signed in to change notification settings - Fork 2.3k
chore: update build appimage script #5866
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
chore: update build appimage script #5866
Conversation
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.
Caution
Changes requested ❌
Reviewed everything up to 0edbf15 in 1 minute and 51 seconds. Click for details.
- Reviewed
19lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src-tauri/build-utils/buildAppImage.sh:6
- Draft comment:
The download URL has changed from using the AppImageKit repository to appimagetool. Confirm this change is intentional. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment violates multiple rules. It's asking for confirmation ("Confirm this change is intentional") which is explicitly forbidden. It's also about a dependency/tool download URL which falls under the dependency-related changes we should ignore. The change appears deliberate and the script still works since it's just downloading the same tool from its new location. Maybe this URL change could break the build process if the new URL is incorrect? Maybe this is a security concern? Build failures would be caught by CI, and the URL is still within the official AppImage organization on GitHub. This is clearly just a repository reorganization. Delete the comment as it violates rules about asking for confirmation and commenting on dependency/tool changes.
Workflow ID: wflow_Yj4k52vumSdPMi7o
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Barecheck - Code coverage reportTotal: 35.16%Your code coverage diff: 0.01% ▴ ✅ All code changes are covered |
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.
Important
Looks good to me! 👍
Reviewed 6dd65ab in 40 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src-tauri/build-utils/buildAppImage.sh:6
- Draft comment:
Good error check added. Consider redirecting the error output to stderr (e.g., using >&2) so that failure messages are clearly treated as errors. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_lEKyDjpVCiGUu4Xr
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
This pull request simplifies the
buildAppImage.shscript by removing unnecessary conditional logic and ensuring theappimagetoolis always downloaded.Key changes to the script:
appimagetoolfile and the associated comment, ensuring that the tool is always downloaded. (src-tauri/build-utils/buildAppImage.sh, src-tauri/build-utils/buildAppImage.shL5-L10)Important
Simplifies
buildAppImage.shby always downloadingappimagetool, removing unnecessary conditional logic.appimagetoolexistence inbuildAppImage.sh, ensuring it is always downloaded.This description was created by
for 6dd65ab. You can customize this summary. It will automatically update as commits are pushed.