fix: sdk release - #222
Conversation
WalkthroughThe release CI workflow was updated: Linux build matrix now targets x86_64 only (aarch64 removed). The workflow also adds explicit Python 3.10 setup steps and installs protoc inside the manylinux/Linux container before building wheels/sdist across several job paths. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as "GitHub Action Runner"
participant Setup as "setup-python (3.10)"
participant Protoc as "in-container protoc (25.1)"
participant Build as "build (wheel/sdist)"
Note over Dev,Setup: Job starts (Linux x86_64 only)
Dev->>Setup: install Python 3.10
Setup-->>Dev: Python ready
Dev->>Protoc: download & install protoc in container
Protoc-->>Dev: protoc ready
Dev->>Build: build wheels/sdist
alt success
Build-->>Dev: artifacts produced
else failure
Build-->>Dev: error logs
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-python-sdk.yml (1)
1-272: PR description is incomplete; it contains only template placeholders with no substantive change context.The PR description lacks critical information for a breaking change PR:
- Summary: "Brief description of what this PR does" (placeholder, not filled)
- Related Issues: Empty (no issue referenced to explain the decision)
- Type of Change: No checkboxes selected (unclear if this is a bug fix vs. intentional breaking change)
- Changes Made: Empty list
- Testing: No items checked; no test verification performed
- Checklist: No formatting, clippy, tests, or documentation checks completed
Please update the PR description with:
- A meaningful summary of why aarch64 Linux support is being removed
- A link to the related issue or decision (e.g., "Closes #XYZ" or "Related to: ")
- Selection of "Breaking change" in Type of Change
- Completion of the pre-merge checklist items
- Confirmation that formatting, clippy, tests, and documentation requirements have been met
♻️ Duplicate comments (1)
.github/workflows/release-python-sdk.yml (1)
262-262: Release notes updated consistently; however, review the breaking change rationale first.The release notes documentation has been updated to reflect the x86_64-only Linux support, which is consistent with the workflow matrix change. However, this change should not be merged until the rationale for removing aarch64 support is documented.
This is consistent with the change at line 53, but both changes should be considered as part of the breaking change concern already flagged above.
| strategy: | ||
| matrix: | ||
| target: [x86_64, aarch64] | ||
| target: [x86_64] |
There was a problem hiding this comment.
🧩 Analysis chain
Breaking change: aarch64 Linux support removed without clear rationale or migration guidance.
The Linux build matrix has been restricted to x86_64 only, removing aarch64 support. This is a significant breaking change that could disrupt users on ARM-based Linux systems. The PR description provides no explanation for this change, no related issue reference, and no migration guidance.
Before approving this change, please provide:
- A clear rationale for removing aarch64 support (is it a build issue, maintenance burden, or intentional platform limitation?)
- A reference to the related issue or ADR (Architectural Decision Record) explaining this decision
- Updated CHANGELOG entry documenting this breaking change
- Migration guidance for users currently using aarch64 Linux wheels
You can verify if there's a related issue or CHANGELOG entry:
🏁 Script executed:
#!/bin/bash
# Check for related issues or PRs
gh pr view 222 --json body,labels
# Search for aarch64 references in CHANGELOG files
fd -e md -e MD CHANGELOG | xargs grep -l "aarch64" || echo "No aarch64 references found in CHANGELOG files"
# Look for recent commits related to aarch64 removal
git log --oneline --all -n 50 | grep -i "aarch64\|arm64" || echo "No recent commits mentioning aarch64/arm64"Length of output: 2011
Breaking change requires proper documentation and rationale before merging.
Verification confirms this PR introduces a breaking change (removing aarch64 Linux support) without the necessary supporting documentation:
- PR description is empty template text with no explanation for the removal
- No related issue referenced
- No CHANGELOG entry documenting the breaking change
- No commit history explaining the decision
Required before approval:
- Fill PR description with actual rationale for removing aarch64 support
- Add CHANGELOG entry documenting this as a breaking change
- Reference related issue/ADR explaining the architectural decision
- Consider providing migration guidance for affected users
🤖 Prompt for AI Agents
In .github/workflows/release-python-sdk.yml around line 53 the workflow was
changed to target only x86_64 which removes aarch64 Linux support; before
merging, update project documentation and metadata: replace the PR template text
with a clear rationale for removing aarch64 (who/why/impact), add a CHANGELOG
entry marking this as a breaking change with affected versions and migration
guidance, link or create an issue/ADR that records the architectural decision
and alternatives considered, and add a short note in the workflow file or commit
message referencing the issue/ADR so reviewers can verify intent.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release-python-sdk.yml (1)
53-53:⚠️ Duplicate: Breaking change requires prior documentation requirements to be met.This represents the same critical issue flagged in the previous review: removing aarch64 Linux support is a breaking change that requires proper documentation before approval.
The prior review required:
- Clear rationale for removing aarch64 support
- Reference to related issue/ADR explaining the decision
- Updated CHANGELOG entry documenting this as a breaking change
- Migration guidance for affected users
Current status: None of these requirements have been addressed in this PR. The PR description remains empty template text, no CHANGELOG entry exists, and no issue reference is provided.
Before proceeding, please:
- Fill the PR description with the actual rationale for removing aarch64 Linux build support
- Add a CHANGELOG entry documenting this as a breaking change (affected versions, impact, migration path)
- Reference or create an issue/ADR documenting the architectural decision
- Update commit message to reference the issue for traceability
Also applies to: 277-277
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release-python-sdk.yml(5 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). (11)
- GitHub Check: build-payments (stable)
- GitHub Check: test-python-sdk (3.12)
- GitHub Check: test-python-sdk (3.13)
- GitHub Check: build-cli (stable)
- GitHub Check: test-python-sdk (3.10)
- GitHub Check: test-python-sdk (3.11)
- GitHub Check: build-billing (stable)
- GitHub Check: build-api (stable)
- GitHub Check: build-validator (stable)
- GitHub Check: build-miner (stable)
- GitHub Check: quick-checks
🔇 Additional comments (3)
.github/workflows/release-python-sdk.yml (3)
64-69: Protoc installation in manylinux container looks sound.The approach of downloading and installing protoc v25.1 directly in the manylinux container is reasonable and ensures availability before the build. Version pinning improves reproducibility.
For context: Is this change addressing a missing protoc dependency that was causing failures, or is it unrelated to the aarch64 removal? Please clarify in the PR description or commit message for future reference.
87-90: Python 3.10 setup is explicit and consistent across jobs.Adding explicit Python 3.10 setup via
actions/setup-python@v5across macOS, Windows, and sdist jobs is good practice. The configuration is consistent and well-formed.Clarification needed: The project supports Python 3.10–3.13 (line 280). Why is Python 3.10 specifically required for the build/release workflow (as opposed to 3.11, 3.12, or 3.13)? Is this a protoc build requirement, a maturin requirement, or something else? Please document this in the PR description or commit message.
Also applies to: 115-118, 143-146
1-12: The review comment cannot be fully verified against only the code provided.Based on the workflow file content and search results, I can confirm:
Platform support state (actual code):
- The workflow builds only
x86_64for Linux—noaarch64-unknown-linux-gnuis present in the build matrix- macOS supports both
x86_64-apple-darwinandaarch64-apple-darwin- Release notes correctly document supported platforms
Protoc installation (valid approach):
- Three OS-specific methods are appropriate: manual install on Linux,
brewon macOS,chocoon Windows- The
arduino/setup-protocaction for sdist is a standard approachPython 3.10 pinning (clear):
- Explicitly set in three jobs; reasonable for reproducibility
However, the review comment's core claim—that aarch64-linux support was removed without documentation—cannot be verified from the workflow file alone. The workflow shows only the current state, not what changed. To properly verify the review comment, I would need:
- PR description (to confirm it's empty template as claimed)
- Commit messages explaining the matrix change
- Previous workflow version (to confirm aarch64-linux was removed)
- Related issues or ADRs
| ## Supported Platforms | ||
|
|
||
| - Linux: x86_64, aarch64 (ARM64) | ||
| - Linux: x86_64 | ||
| - macOS: x86_64 (Intel), aarch64 (Apple Silicon) | ||
| - Windows: x86_64 | ||
| - Python: 3.10, 3.11, 3.12, 3.13 |
There was a problem hiding this comment.
Release notes need breaking-change highlight for aarch64 removal.
The release notes correctly show "Linux: x86_64" but lack any callout that aarch64 Linux support has been removed. Users on ARM-based Linux systems will see this without explanation.
Recommendation: Add a "
🤖 Prompt for AI Agents
.github/workflows/release-python-sdk.yml around lines 275–280: the release notes
list supported platforms but omit a prominent notice that Linux aarch64 support
was removed; add a "⚠️ Breaking Changes" section near line 282 (before or after
"What's Changed") that clearly states the removal of aarch64 Linux support,
explains impact on ARM-based Linux users, and provides brief migration guidance
or alternatives (e.g., use x86_64 builds under emulation, run on supported
x86_64 hardware, or contact support/roadmap for aarch64 reinstatement).
2 participants
Summary
Brief description of what this PR does.
Related Issues
Closes #(issue number)
Type of Change
Changes Made
List the main changes in this PR:
Testing
How Has This Been Tested?
Describe the tests you ran to verify your changes.
cargo test)Test Configuration
Checklist
cargo fmtto format my codecargo clippyand addressed all warningsAdditional Context
Add any other context about the PR here.
Summary by CodeRabbit