[v3] Update Templates (macOS Target Version and Linux aarch64 AppImage) #3981
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces changes to the build and configuration files for cross-platform application deployment. The modifications include architecture-specific handling for the Linux AppImage build script and an update to the minimum macOS version requirement for both development and production Info.plist templates. Additionally, the changelog has been updated to reflect these changes, enhancing compatibility and support for different system architectures and newer macOS versions. Changes
Sequence DiagramsequenceDiagram
participant Script as Build Script
participant System as System Architecture
participant Linuxdeploy as Linuxdeploy Tool
Script->>System: Detect architecture
alt x86_64 Architecture
Script->>Linuxdeploy: Download x86_64 version
else ARM64 Architecture
Script->>Linuxdeploy: Download arm64 version
end
Script->>Linuxdeploy: Execute tool
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
AkshayKalose
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
v3/internal/commands/updatable_build_assets/darwin/Info.plist.tmpl (1)
21-21: Acknowledge the new minimum macOS version requirement.Raising
LSMinimumSystemVersionfrom10.13.0to10.15.0will exclude users running older macOS versions (10.13–10.14). Ensure that this updated requirement aligns with your supported target audience.v3/internal/commands/build_assets/linux/appimage/build.sh (2)
17-20: Consider verifying the downloaded file for security & integrity.While this logic appropriately handles x86_64 architecture, it uses
wgetwithout checksum validation. Malicious or incomplete downloads are not detected.You could do:
if [[ $(uname -m) == *x86_64* ]]; then + # Optionally verify the download via checksums wget -q -4 -N https://github.com/linuxdeploy/linuxdeploy/releases/download/continuous/linuxdeploy-x86_64.AppImage chmod +x linuxdeploy-x86_64.AppImage
25-31: Great addition of arm64 support.This enables building for a broader range of Linux platforms. Consider implementing the same security checks and bundle verifications for arm64 that you do for x86_64 (e.g., checksums, debugging logs).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v3/internal/commands/build_assets/linux/appimage/build.sh(1 hunks)v3/internal/commands/updatable_build_assets/darwin/Info.dev.plist.tmpl(1 hunks)v3/internal/commands/updatable_build_assets/darwin/Info.plist.tmpl(1 hunks)
🔇 Additional comments (2)
v3/internal/commands/updatable_build_assets/darwin/Info.dev.plist.tmpl (1)
21-21: Verify consistency with the production plist.Mirroring the production plist’s requirement in the dev plist helps maintain a uniform target across development and production. Confirm that your dev users have at least macOS 10.15 installed.
v3/internal/commands/build_assets/linux/appimage/build.sh (1)
22-23: Confirm correct app bundling for x86_64.Remember to verify that
linuxdeploy-x86_64.AppImageproperly includes all critical files and dependencies for your application bundle.
|
Thanks 🙏 Please could you add an entry to the changelog located at |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/src/content/docs/changelog.mdx (1)
60-60: Consider highlighting this as a breaking change and fix grammar.This update to the minimum macOS system version requirement is a breaking change that could affect users with older systems. Additionally, there's a minor grammatical issue.
Apply this diff to improve the entry:
- Updated the minimum system version in macOS .plist files from 10.13.0 to 10.15.0 + Updated the minimum system version on macOS .plist files from 10.13.0 to 10.15.0 (Breaking Change)🧰 Tools
🪛 LanguageTool
[grammar] ~60-~60: The correct preposition appears to be “on”.
Context: ...2) - Updated the minimum system version in macOS .plist files from 10.13.0 to 10.1...(IN_WINDOWS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/content/docs/changelog.mdx(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/content/docs/changelog.mdx
[grammar] ~60-~60: The correct preposition appears to be “on”.
Context: ...2) - Updated the minimum system version in macOS .plist files from 10.13.0 to 10.1...
(IN_WINDOWS)
🔇 Additional comments (1)
docs/src/content/docs/changelog.mdx (1)
40-40: LGTM! Well-documented feature addition.The changelog entry for aarch64 (arm64) AppImage Linux builds support is clear, properly attributed, and follows the changelog format guidelines.
|
Thank you! 🙏 |
The changes are tested and working on the repo: https://github.com/grpcmd/grpcmd-gui
Summary by CodeRabbit
linuxdeploytool download.