-
Notifications
You must be signed in to change notification settings - Fork 27
Release 2.1.1 #631
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
Release 2.1.1 #631
Conversation
WalkthroughRelease workflow updated to add artifact validation for the ably/sync/ path in wheel and sdist. Poetry setup version adjusted. Package version bumped to 2.1.1 across pyproject and init. Changelog updated with v2.1.1 entry noting inclusion of the sync folder in the wheel. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Maintainer
participant GH as GitHub Actions: release.yml
participant Build as Build & Package
participant Check as Artifact Validator
participant Registry as Package Registry
Dev->>GH: Push tag / trigger release
GH->>Build: Install Poetry (setup 1.8.5)
Build->>Build: Build wheel and sdist
Build-->>GH: Upload artifacts
GH->>Check: Inspect wheel for ably/sync/
Check-->>GH: Pass/Fail
GH->>Check: Inspect sdist for ably/sync/
Check-->>GH: Pass/Fail
alt Validation passes
GH->>Registry: Publish artifacts
Registry-->>Dev: Release available
else Validation fails
GH-->>Dev: Workflow fails early
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
9382a1b to
84d5b78
Compare
84d5b78 to
de6c2f5
Compare
de6c2f5 to
63c95ab
Compare
63c95ab to
546caa9
Compare
546caa9 to
97a25c7
Compare
Newest releases of poetry somehow miss the ` sync ` folder. Added tests that check that `sync` exists
97a25c7 to
4121b76
Compare
4121b76 to
7e67dc3
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pyproject.toml (1)
26-28: Prefer Poetry “packages” over “include” for reliable packaging of generated sync modules.Using “include = ['ably/**/*.py']” is non‑idiomatic and may be brittle for generated code. Let Poetry discover the ably package explicitly.
Apply this diff:
-include = [ - 'ably/**/*.py' -] +packages = [{ include = "ably" }]Please confirm ably/sync has init.py files after unasync so it’s recognized as a package at build time.
CHANGELOG.md (1)
3-10: Trim trailing whitespace.Minor nit on Line 9.
-* Added missed `sync` folder to the wheel package +* Added missed `sync` folder to the wheel package.github/workflows/release.yml (1)
59-79: Harden the artifact validation step (safer globbing, strict mode, clearer failures).Prevents false negatives when no files are present and improves diagnostics. Keeps unzip/tar approach.
- - name: Check that wheel and tarball contains ably/sync/ - run: | - # Check wheel - WHEEL=$(ls dist/*.whl | head -n 1) - echo "Checking wheel: $WHEEL" - if unzip -l "$WHEEL" | grep -q "ably/sync/"; then - echo "✅ Found ably/sync/ in wheel" - else - echo "❌ ably/sync/ not found in wheel" - exit 1 - fi - - # Check tarball - TARBALL=$(ls dist/*.tar.gz | head -n 1) - echo "Checking tarball: $TARBALL" - if tar -tzf "$TARBALL" | grep -q "ably/sync/"; then - echo "✅ Found ably/sync/ in tarball" - else - echo "❌ ably/sync/ not found in tarball" - exit 1 - fi + - name: Check that wheel and tarball contain ably/sync/ + shell: bash + run: | + set -euo pipefail + + # Check wheel + shopt -s nullglob || true + wheels=(dist/*.whl) + if [ ${#wheels[@]} -eq 0 ]; then + echo "❌ No wheel files found in dist/" + exit 1 + fi + WHEEL="${wheels[0]}" + echo "Checking wheel: $WHEEL" + if unzip -l "$WHEEL" | grep -q 'ably/sync/'; then + echo "✅ Found ably/sync/ in wheel" + else + echo "❌ ably/sync/ not found in wheel" + exit 1 + fi + + # Check tarball + tarballs=(dist/*.tar.gz) + if [ ${#tarballs[@]} -eq 0 ]; then + echo "❌ No tarball files found in dist/" + exit 1 + fi + TARBALL="${tarballs[0]}" + echo "Checking tarball: $TARBALL" + if tar -tzf "$TARBALL" | grep -q 'ably/sync/'; then + echo "✅ Found ably/sync/ in tarball" + else + echo "❌ ably/sync/ not found in tarball" + exit 1 + fiOptional: Move this check before the upload-artifact step to fail earlier in the build job.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/release.yml(2 hunks)CHANGELOG.md(1 hunks)ably/__init__.py(1 hunks)pyproject.toml(1 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). (7)
- GitHub Check: check (3.13)
- GitHub Check: check (3.10)
- GitHub Check: check (3.9)
- GitHub Check: check (3.8)
- GitHub Check: check (3.7)
- GitHub Check: check (3.11)
- GitHub Check: check (3.12)
🔇 Additional comments (3)
ably/__init__.py (1)
19-19: Version bump aligned with release.Matches pyproject and changelog updates.
pyproject.toml (1)
3-3: Version bump looks good.Consistent with ably/init.py and the release workflow.
.github/workflows/release.yml (1)
27-27: Pinning Poetry 1.8.5 is fine.Good for reproducibility. Ensure lockfile compatibility if regenerated locally with a different Poetry version.
sacOO7
left a comment
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.
Lgtm
Resolves #630
Added missed
syncfolder to the wheel packageSummary by CodeRabbit