ci: arm builds for linux #79
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 6607557 in 2 minutes and 5 seconds
More details
- Looked at
37lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. .github/workflows/release.yml:41
- Draft comment:
Duplicate step name: Rename one of the "Install dependencies (ubuntu only)" steps to reflect platform differences. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. .github/workflows/release.yml:26
- Draft comment:
The comment for the Windows runner says 'for Arm based windows' but the args target x86_64. Clarify or update the comment to avoid confusion. - Reason this comment was not posted:
Marked as duplicate.
3. .github/workflows/release.yml:34
- Draft comment:
Duplicate step names for installing Ubuntu dependencies can be confusing. Consider renaming one (e.g., ‘ubuntu-22.04’ vs. ‘ubuntu-22.04-arm’). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. .github/workflows/release.yml:39
- Draft comment:
Inconsistent indentation in the run block (e.g., between 'sudo apt-get update' and the following install command) may affect readability. Align the indents consistently. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
Workflow ID: wflow_bF4Ywi4hn7IPVpCI
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
.github/workflows/release.yml
Outdated
| args: '' | ||
| - platform: 'ubuntu-22.04-arm' # for Arm based linux. | ||
| args: '' | ||
| - platform: 'windows-latest' # for Arm based windows. |
There was a problem hiding this comment.
Mismatch: comment says "for Arm based windows" but target is x86_64. Clarify or correct.
| - platform: 'windows-latest' # for Arm based windows. | |
| - platform: 'windows-latest' # for Intel/AMD based windows. |
.github/workflows/release.yml
Outdated
| args: '--target x86_64-apple-darwin' | ||
| - platform: 'ubuntu-22.04' | ||
| args: '' | ||
| - platform: 'ubuntu-22.04-arm' # for Arm based linux. |
There was a problem hiding this comment.
Runner label 'ubuntu-22.04-arm' may not match a GitHub-hosted runner. Ensure a self-hosted runner is configured or use a valid label.
.github/workflows/release.yml
Outdated
| @@ -14,7 +14,17 @@ jobs: | |||
| strategy: | |||
| fail-fast: false | |||
| matrix: | |||
| platform: [macos-latest, ubuntu-22.04, windows-latest] | |||
| include: | |||
There was a problem hiding this comment.
The matrix entries define an 'args' field for each platform but it's not used in any subsequent steps. Integrate it where needed or remove it if unnecessary.
0xbrayo
force-pushed
the
aarch64
branch
3 times, most recently
from
dca38cd to
6a2737a
Compare
|
@ellipsis-dev review |
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 6a2737a in 2 minutes and 49 seconds
More details
- Looked at
71lines of code in2files - Skipped
0files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. .github/workflows/release.yml:41
-
Draft comment:
Consider extracting this step to a reusable workflow or composite action to avoid duplication. -
Step: Install dependencies (ubuntu-arm only) in build workflow (build.yml)
-
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment points out real duplication between two workflow files. The suggestion to use a reusable workflow or composite action is a concrete, actionable solution. This is a legitimate code quality improvement that would reduce maintenance burden. However, this kind of cross-file refactoring suggestion violates the rule about ignoring cross-file issues.
The suggestion requires looking at multiple files to understand and implement. The duplication might be intentional to keep the workflows independent.
While the suggestion is valid, our rules explicitly state to ignore cross-file issues and only think about the file being reviewed.
Due to the rule about ignoring cross-file issues, this comment should be removed despite being technically correct.
2. .github/workflows/build.yml:43
- Draft comment:
In the ubuntu-arm dependency block, the second command appears to have an extra leading space. Ensure consistent indentation for the multiline script. - 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%
While the comment is technically correct about the inconsistent indentation, this is a very minor formatting issue in a GitHub Actions workflow file. The extra space won't affect the functionality of the shell commands. YAML is sensitive to indentation, but within therun: |block, the shell commands' indentation is not significant for YAML parsing.
The inconsistent indentation could potentially indicate a copy-paste error that might hide other issues. Also, consistent formatting helps with maintainability.
While consistency is good, this is an extremely minor issue that doesn't affect functionality or readability significantly. The CI/CD pipeline will work exactly the same either way.
This comment should be deleted as it points out an extremely minor formatting issue that doesn't impact functionality or significantly affect readability.
3. .github/workflows/release.yml:46
- Draft comment:
The ubuntu-arm installation step has extra leading space on the second command line. Please fix the indentation for consistency. - Reason this comment was not posted:
Marked as duplicate.
4. .github/workflows/build.yml:43
- Draft comment:
Inconsistent indentation in the run block for installing ubuntu-arm dependencies; align command lines to avoid accidental whitespace. - Reason this comment was not posted:
Marked as duplicate.
5. .github/workflows/release.yml:26
- Draft comment:
The comment for 'windows-latest' is misleading: it says 'for Arm based windows' while the args target x86_64. Update the comment or target for consistency. - Reason this comment was not posted:
Marked as duplicate.
6. .github/workflows/release.yml:39
- Draft comment:
Inconsistent indentation in the run block for installing Ubuntu dependencies; ensure consistent alignment of all command lines. - Reason this comment was not posted:
Marked as duplicate.
7. .github/workflows/release.yml:68
- Draft comment:
Matrix 'args' is defined in the strategy but not used in the tauri-action build step. Consider passing ${{ matrix.args }} if it's needed. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_N01yBv2WHIb0ueM0
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
.github/workflows/release.yml
Outdated
| args: '' | ||
| - platform: 'ubuntu-22.04-arm' # for Arm based linux. | ||
| args: '' | ||
| - platform: 'windows-latest' # for Arm based windows. |
There was a problem hiding this comment.
The comment '# for Arm based windows' is misleading since the args target is x86_64. Update comment or verify the intended target.
| - platform: 'windows-latest' # for Arm based windows. | |
| - platform: 'windows-latest' # for x86_64 based windows. |
.github/workflows/build.yml
Outdated
| platform: [macos-latest, ubuntu-22.04, windows-latest] | ||
| include: | ||
| - platform: 'macos-latest' # for Arm based macs | ||
| args: '--target aarch64-apple-darwin' |
There was a problem hiding this comment.
Matrix 'args' is defined but not used later. Consider passing ${{ matrix.args }} to the tauri-action step if needed.
|
@ellipsis-dev review |
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 597f06a in 1 minute and 47 seconds
More details
- Looked at
82lines of code in2files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. .github/workflows/build.yml:42
- Draft comment:
The indentation for the second command in the ubuntu-arm dependency installation appears off. Please fix the indentation so that both commands align correctly. - Reason this comment was not posted:
Marked as duplicate.
2. .github/workflows/release.yml:39
- Draft comment:
There’s an indentation issue in the ubuntu-arm dependency installation block. Adjust the indentation for the 'sudo apt-get install' command. - Reason this comment was not posted:
Marked as duplicate.
3. .github/workflows/build.yml:21
- Draft comment:
Explicitly set an empty args value for the 'ubuntu-22.04' entry for consistency. - 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%
The lack of args for ubuntu-22.04 is intentional - it means use default target. Adding empty args would be purely cosmetic and wouldn't change behavior. The current format is clear and follows common YAML practices. Having explicit empty values could actually reduce readability.
Perhaps having consistent structure across all matrix entries would make it easier to add new targets in the future? Maybe explicit empty values are preferred in some style guides?
The current format is perfectly valid YAML and common practice. Adding empty values would be purely cosmetic with no functional benefit and could make the config harder to read.
This comment should be deleted as it suggests a purely cosmetic change that doesn't improve code quality or functionality.
4. .github/workflows/build.yml:43
- Draft comment:
Fix indentation in the ubuntu-arm dependency install step; the 'sudo apt-get install' line is misaligned. - Reason this comment was not posted:
Marked as duplicate.
5. .github/workflows/release.yml:46
- Draft comment:
Correct the YAML indentation in the ubuntu-arm dependency block; ensure consistent spacing for all command lines. - Reason this comment was not posted:
Comment looked like it was already resolved.
6. .github/workflows/release.yml:27
- Draft comment:
Update the comment for the 'windows-latest' matrix entry; the args indicate an x86_64 target, so the comment should reflect that. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_Vojjq02L08wVqHc1
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
.github/workflows/release.yml
Outdated
| args: '' | ||
| - platform: 'ubuntu-22.04-arm' # for Arm based linux. | ||
| args: '' | ||
| - platform: 'windows-latest' # for Arm based windows. |
There was a problem hiding this comment.
The comment for the 'windows-latest' platform suggests it's for Arm based Windows, but the target argument '--target x86_64-pc-windows-msvc' is for x86_64 builds. Please clarify this inconsistency.
| - platform: 'windows-latest' # for Arm based windows. | |
| - platform: 'windows-latest' # for x86_64 based windows. |
0xbrayo
force-pushed
the
aarch64
branch
5 times, most recently
from
1780e07 to
f3fbff8
Compare
|
@ellipsis-dev review |
#73
Important
Add ARM build support for macOS and Linux in GitHub Actions workflows.
build.ymlandrelease.yml.macos-latestwith--target aarch64-apple-darwinandubuntu-22.04-arm.argsparameter totauri-actionstep to pass target architecture.ubuntu-22.04-armin both workflows.This description was created by
for 597f06a. It will automatically update as commits are pushed.