Skip to content

Conversation

@Eeems
Copy link
Collaborator

@Eeems Eeems commented Sep 26, 2024

Summary by CodeRabbit

  • Chores
    • Added support for macOS 15 (Intel) in CI matrices and adjusted package-install logic to include it.
    • Release workflow now waits for an additional test path.
    • Added a CI step to free up disk space during device tests.
    • Renamed a compilation artifact output for clarity.
    • Standardized CI quoting/formatting and minor condition/whitespace fixes.
    • Bumped two platform-specific helper packages to v1.3.1.

✏️ Tip: You can customize this high-level summary in your review settings.

@Eeems Eeems mentioned this pull request Sep 26, 2024
@Eeems Eeems linked an issue Sep 26, 2024 that may be closed by this pull request
@Eeems

This comment was marked as resolved.

@Eeems Eeems changed the title Also build on macos-13 as it's intel Add intel macOS build Dec 12, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 12, 2025

Walkthrough

Added macos-15-intel to CI job matrices and brew-install condition, standardized string quoting, renamed an artifact output, added a "Free up space" step, adjusted job needs, and bumped two dependency versions in requirements.txt to 1.3.1 (platform markers preserved).

Changes

Cohort / File(s) Summary
Workflow & CI configuration
\.github/workflows/main.yml
Added macos-15-intel to job matrices (remote/test paths); expanded brew-install if to include macos-15-intel; added "Free up space" step under remote/test_device; renamed artifact output to remarkable-compilation-report; standardized many string literals to double quotes and applied minor whitespace/formatting tweaks; updated release job needs to include test_device.
Dependencies
requirements.txt
Bumped remarkable-update-image and remarkable-update-fuse from 1.3 to 1.3.1, preserving existing platform markers (sys_platform != 'linux' and sys_platform == 'linux').

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Inspect YAML syntax/indentation around the matrix additions and modified if conditions.
  • Verify the new "Free up space" step placement and the release job needs change.
  • Confirm PEP 508 formatting for platform markers and version pins in requirements.txt.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add intel macOS build' directly describes the main change in the changeset: adding macos-15-intel to the workflow matrix and related configurations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Eeems-patch-1

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e63248 and 846bd60.

📒 Files selected for processing (1)
  • .github/workflows/main.yml (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/main.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build for macos-latest
  • GitHub Check: Build for macos-15-intel
  • GitHub Check: Build for ubuntu-latest
  • GitHub Check: Build for remarkable
  • GitHub Check: Build for windows-latest

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Eeems
Copy link
Collaborator Author

Eeems commented Dec 12, 2025

I think this is ready, both x86_64 and arm64 mach-o binaries are being generated. Based on the automated testing we do as part of the build, I'm fairly comfortable with saying this is probably working and can be merged. Any concerns @Jayy001 ?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/main.yml (1)

165-171: Bug: jobs.device references ${{ matrix.os }} but device has no matrix.
This will break artifact naming (and may error during expression evaluation). Use a fixed name (e.g., remarkable-compilation-report) or ${{ github.job }}.

       - name: Upload Compilation Report
         uses: actions/upload-artifact@v4
         if: runner.debug == '1'
         with:
-          name: ${{ matrix.os }}-compilation-report
+          name: remarkable-compilation-report
           path: compilation-report.xml
           if-no-files-found: warn
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)

193-208: Disk cleanup step is reasonable; consider adding sudo apt-get clean -y for a bit more space (optional).
Current step is fine as-is.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e6e461 and 6e63248.

📒 Files selected for processing (1)
  • .github/workflows/main.yml (7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build for ubuntu-latest
  • GitHub Check: Build for windows-latest
  • GitHub Check: Build for macos-15-intel
  • GitHub Check: Build for macos-latest
  • GitHub Check: Build for remarkable
🔇 Additional comments (7)
.github/workflows/main.yml (7)

7-15: Workflow triggers now include .github/workflows/main.yml changes (good), but verify path globs are intended for your repo layout.
If this repo relies on files outside these globs (e.g., non-.py sources, pyproject.toml, etc.), CI may not run when it should.

Also applies to: 20-28


56-58: Brew install condition looks correct, contingent on macos-15-intel actually existing.
No further concerns here.


72-75: setup-python quoting changes are fine; pinning Python 3.12 is clear and stable.
No issues.


88-94: Artifact upload if is now robust (uploads on success/failure when debug enabled).
Good for diagnostics without failing the job.


184-187: Quoting fw_version strings is a good fix (prevents YAML numeric coercion).
No issues.


214-215: Verify the new codexctl download invocation is correct (--out /tmp toltec).
This looks like an argument-order change; if toltec is meant to be a positional sub-arg, ensure the CLI parses it as intended (and that /tmp exists/has space on the runner + in the reMarkable action environment).


218-218: Release now correctly waits on test_device (stronger gate), but ensure you want releases blocked if test_device is later expanded/flaky.
Consider continue-on-error for test_device only if you want “soft” gating.

@Eeems Eeems merged commit c2d44b9 into main Dec 13, 2025
9 checks passed
@Eeems Eeems deleted the Eeems-patch-1 branch December 13, 2025 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mac build is arm only

2 participants