-
Notifications
You must be signed in to change notification settings - Fork 5
Automate release & uploading artifacts #904
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
Conversation
📝 WalkthroughWalkthroughCentralizes CI and release orchestration via a new Changes
Sequence DiagramsequenceDiagram
autonumber
actor Developer
participant CI as CI Orchestrator (ci.yml)
participant Prep as Release Workflow (release.yml)
participant Build as Build Workflow (build.yml)
participant Run as Test Workflow (run.yml)
participant GH as GitHub Releases
Developer->>CI: push / tag / manual dispatch
CI->>Prep: workflow_call (prep-release)
Prep->>GH: create/update draft release (generate notes, attach LICENSE)
Prep-->>CI: outputs.release_id
par Build and Test
CI->>Build: workflow_call (with release_id)
CI->>Run: workflow_call (with/without release_id as configured)
Build->>Build: build wheel, SBOM, binaries, installers
Build->>GH: upload installers & wheel to draft release (if release_id present)
Run->>Run: execute test matrix
end
GH-->>Developer: draft release populated with artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (16)
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 |
1ad1d6b to
01c308c
Compare
01c308c to
af0cc3c
Compare
956203e to
49ceb37
Compare
1f44736 to
332afaf
Compare
332afaf to
3c146a7
Compare
1957cc1 to
6a08dd3
Compare
6a08dd3 to
4a259cc
Compare
c017d02 to
287f91f
Compare
287f91f to
53cfa6b
Compare
53cfa6b to
155827c
Compare
8426759 to
6d11b24
Compare
6ba1c96 to
45696fe
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: 10
🧹 Nitpick comments (6)
.github/workflows/landing-page.yml (1)
16-22: Consider retaining step names for workflow log clarity.Removing the step names for "Harden the runner" and "Setup Python" reduces readability in GitHub Actions logs, making it harder to quickly identify steps during debugging. While functionally equivalent, explicit names improve maintainability.
.github/workflows/docs.yml (1)
12-16: Consider retaining step names for workflow log clarity.Removing explicit step names reduces readability in GitHub Actions logs, making troubleshooting less efficient. While functionally identical, named steps improve maintainability.
.github/workflows/test.yml (1)
17-21: Consider retaining step names for workflow log clarity.Removing step names ("Harden the runner", "Setup Python") makes GitHub Actions logs less descriptive. Named steps help identify failures quickly during CI runs and improve overall workflow maintainability.
.github/workflows/ci.yml (1)
7-8: Refine tag pattern to match semantic versions only.The current pattern
'*.*.*'will match any three dot-separated segments, including non-version tags like'test.1.2'or'fix.bug.now'. Consider using a more specific pattern that enforces numeric semantic versioning.🔎 Suggested improvement
tags: - - '*.*.*' + - '[0-9]+.[0-9]+.[0-9]+'script/create_release_notes.py (1)
31-37: Consider clarifying the end_idx=None behavior.When only one release section exists,
end_idxremainsNone, and the slicelines[start_idx:None]correctly captures all remaining lines. While this works correctly in Python, the implicit behavior could be made more explicit for maintainability.🔎 Optional clarification
for idx in range(start_idx + 1, len(lines)): if version_header_pattern.match(lines[idx].strip()): end_idx = idx break + # If end_idx is None, capture all lines to the end (single release section) section_lines = lines[start_idx:end_idx] return "\n".join(section_lines).strip().github/workflows/build.yml (1)
40-42: Consider version pinning for Zig installation.Similar to the installation in run.yml, the Zig version is not pinned, which could lead to build inconsistencies over time as new versions are released.
🔎 Suggested improvement
- name: Install Zig (Windows) if: runner.os == 'Windows' - run: choco install zig -y + run: choco install zig --version=0.13.0 -y
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/build.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/landing-page.yml.github/workflows/python-publish.yml.github/workflows/release.yml.github/workflows/run.yml.github/workflows/test.yml.gitignoreCHANGELOG.rstREADME.mdpyproject.tomlscript/create_release_notes.pyscript/create_sbom.pyscript/package.py
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-25T23:53:05.742Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: .github/workflows/build.yml:120-125
Timestamp: 2025-12-25T23:53:05.742Z
Learning: When building macOS PKG installers with fpm in CI, account for the known issue (fpm #1996) where --prefix can be duplicated (e.g., /opt/dfetch becomes /opt/dfetch/opt/dfetch). In the workflow at .github/workflows/build.yml, verify install paths and PATH entries do not assume a single-prefix layout. Adjust packaging scripts or fpm arguments to normalize the final install location and update PATH references accordingly. Add a test step to validate the expected runtime paths after installation.
Applied to files:
.github/workflows/build.yml.github/workflows/ci.yml.github/workflows/release.yml.github/workflows/test.yml.github/workflows/landing-page.yml.github/workflows/run.yml.github/workflows/docs.yml.github/workflows/python-publish.yml
📚 Learning: 2025-12-25T22:46:27.554Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: dfetch/resources/__init__.py:12-17
Timestamp: 2025-12-25T22:46:27.554Z
Learning: In dfetch, when building standalone binaries with Nuitka, resources are packaged at the top level as "resources" (not "dfetch.resources") to avoid name clashes with the binary executable named "dfetch". The code in dfetch/resources/__init__.py uses `"resources"` string when `__compiled__` is detected, which correctly references the top-level resources folder in the standalone build.
Applied to files:
script/create_sbom.py
📚 Learning: 2025-12-25T23:53:09.596Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: .github/workflows/build.yml:120-125
Timestamp: 2025-12-25T23:53:09.596Z
Learning: When using fpm to create macOS PKG installers, there is a known issue (fpm #1996) where the --prefix value is duplicated during installation. For example, with --prefix=/opt/dfetch, files are installed to /opt/dfetch/opt/dfetch instead of /opt/dfetch. This means PATH entries and references should account for this duplication.
Applied to files:
script/package.py
📚 Learning: 2025-12-25T23:52:41.740Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: .github/workflows/build.yml:120-125
Timestamp: 2025-12-25T23:52:41.740Z
Learning: In macOS PKG installers created with fpm for the dfetch project, the installation path `/opt/dfetch/opt/dfetch` is correct and intentional due to fpm's osxpkg prefix behavior (related to fpm issue #1996). This apparent path duplication should not be flagged as an error.
Applied to files:
script/package.py
📚 Learning: 2025-12-15T20:30:35.254Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 865
File: .readthedocs.yml:19-19
Timestamp: 2025-12-15T20:30:35.254Z
Learning: Read the Docs supports Python 3.13 and 3.14 (and newer versions). The build.tools.python configuration in .readthedocs.yml accepts version strings like "3.13", "3.14", or "latest" to use the newest available Python version.
Applied to files:
CHANGELOG.rst
🪛 actionlint (1.7.9)
.github/workflows/release.yml
52-52: if: condition "${{ steps.release_info.outputs.tag }} == 'latest'" is always evaluated to true because extra characters are around ${{ }}
(if-cond)
.github/workflows/python-publish.yml
98-98: property "release_id" is not defined in object type {}
(expression)
112-112: property "release_id" is not defined in object type {}
(expression)
⏰ 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). (2)
- GitHub Check: build / build (windows-latest)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (12)
.gitignore (1)
16-16: LGTM!Ignoring the generated release notes file is appropriate and consistent with other build artifacts already in the ignore list.
.github/workflows/docs.yml (1)
18-18: Python version pinned from '3.x' to '3.13'.This changes behavior from using the latest Python 3.x to specifically 3.13. While this improves consistency across workflows (matching other files in this PR), ensure documentation builds are compatible with 3.13 and consider whether automatic adoption of future 3.x versions was intentional.
.github/workflows/python-publish.yml (1)
8-8: LGTM! Appropriate trigger for production releases.Changing from
createdtopublishedensures the workflow only runs when releases are actually published, not when they're created as drafts.script/create_sbom.py (2)
19-24: LGTM! Consistent platform naming.The platform naming convention (nix/osx/win) is consistent with
script/package.pyand provides clear identification for cross-platform artifacts.
46-58: Dynamic version retrieval is correctly implemented.This approach properly retrieves the version after dfetch is installed in the temporary venv. Using
importlib.metadata.versionis the standard method for getting an installed package's version, which will reflect the setuptools-scm computed version.script/package.py (1)
48-53: LGTM! Consistent platform naming.The PLATFORM_NAME logic matches
script/create_sbom.py, ensuring consistent artifact naming across the build process.README.md (1)
57-57: Documentation updated to reflect new release workflow.The change accurately reflects that installers are now available on the releases page, aligning with the automated release workflow introduced in this PR.
CHANGELOG.rst (1)
4-6: LGTM! Clear indication of unreleased status.The note clearly communicates that this is the latest unreleased version, which is helpful for users and aligns with the automated release process.
.github/workflows/ci.yml (1)
25-41: LGTM: Efficient parallel execution design.The
buildandrunjobs correctly depend only onprep-releaseand execute in parallel, which is appropriate since the run job performs testing that doesn't require build artifacts. This design optimizes CI execution time..github/workflows/build.yml (2)
30-33: LGTM: Good security and versioning practices.The addition of
fetch-depth: 0is essential for setuptools-scm dynamic versioning (as configured in pyproject.toml), andpersist-credentials: falsefollows security best practices by not persisting GitHub tokens in git config.
173-222: LGTM: Well-structured wheel build and release jobs.The
build-whlandreleasejobs follow good practices:
- Proper dependency chain (
releasedepends onbuild-whl)- Appropriate security hardening and credential handling
- Clean artifact passing via upload/download-artifact actions
- Consistent versioning support with
fetch-depth: 0.github/workflows/run.yml (1)
38-39: Remove unnecessary Zig installation from Windows jobs.Zig is installed in the workflow but never used in any subsequent test steps. The dfetch project is a Python-based dependency manager with no dependency on Zig. After the installation, only
dfetchcommands are executed, which do not require Zig. This appears to be a debugging artifact or copy-paste error and should be removed to reduce build time and complexity.Likely an incorrect or invalid review 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.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/release.yml (2)
78-88: Draft release blocks automatic PyPI publishing.The workflow creates a draft release but never publishes it. Since
python-publish.ymltriggers only onrelease.types: [published], the PyPI package won't be published automatically. This appears intentional for manual review before publishing, but should be documented.Is the draft-then-manual-publish workflow intentional? If so, consider adding a comment explaining this design choice.
41-42: Remove hardcoded PR branch before merging.The special case for branch
"automate-release"appears to be for testing this PR. This should be removed before merging to avoid treating future pushes to this branch as release candidates.🔎 Suggested cleanup
- if [[ "$BRANCH" == "main" || "$BRANCH" == "automate-release" ]]; then + if [[ "$BRANCH" == "main" ]]; then TAG="latest"script/package.py (1)
10-18: Importingdfetch.__version__may fail when dfetch is not installed.Line 12 imports
__version__from thedfetchpackage, which requires dfetch to be installed and importable. This script runs after building standalone binaries with Nuitka, where dfetch may not be installed as a Python package.Consider using
setuptools_scmconsistently for both version strings, as suggested in the previous review.
🧹 Nitpick comments (1)
script/create_release_notes.py (1)
57-59: UncaughtValueErrorfromextract_latest_sectionwill crash with a traceback.If no release section is found in the changelog (line 30 raises
ValueError), the exception propagates uncaught, producing a stack trace instead of a clean error message. While this does fail the script (non-zero exit), consider catching it for cleaner CI output.🔎 Optional improvement
+ try: + notes = extract_latest_section(changelog_path) + except ValueError as e: + print(f"Error: {e}") + sys.exit(1) + Path("release_notes.txt").write_text( - extract_latest_section(changelog_path), encoding="UTF-8" + notes, encoding="UTF-8" )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/build.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/python-publish.yml.github/workflows/release.yml.github/workflows/run.ymldoc/contributing.rstpyproject.tomlscript/create_release_notes.pyscript/package.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/docs.yml
- .github/workflows/python-publish.yml
- pyproject.toml
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-25T23:53:05.742Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: .github/workflows/build.yml:120-125
Timestamp: 2025-12-25T23:53:05.742Z
Learning: When building macOS PKG installers with fpm in CI, account for the known issue (fpm #1996) where --prefix can be duplicated (e.g., /opt/dfetch becomes /opt/dfetch/opt/dfetch). In the workflow at .github/workflows/build.yml, verify install paths and PATH entries do not assume a single-prefix layout. Adjust packaging scripts or fpm arguments to normalize the final install location and update PATH references accordingly. Add a test step to validate the expected runtime paths after installation.
Applied to files:
.github/workflows/run.yml.github/workflows/release.yml.github/workflows/ci.yml.github/workflows/build.yml
📚 Learning: 2026-01-01T11:05:39.533Z
Learnt from: ben-edna
Repo: dfetch-org/dfetch PR: 898
File: script/package.py:201-205
Timestamp: 2026-01-01T11:05:39.533Z
Learning: In the dfetch project's script/package.py, prefer explicit failures ("crash and burn") over silently handling missing configuration values, ensuring that builds fail if required metadata (e.g., URLs in pyproject.toml) is not properly configured.
Applied to files:
script/package.py
📚 Learning: 2025-12-25T23:52:41.740Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: .github/workflows/build.yml:120-125
Timestamp: 2025-12-25T23:52:41.740Z
Learning: In macOS PKG installers created with fpm for the dfetch project, the installation path `/opt/dfetch/opt/dfetch` is correct and intentional due to fpm's osxpkg prefix behavior (related to fpm issue #1996). This apparent path duplication should not be flagged as an error.
Applied to files:
script/package.py
📚 Learning: 2025-12-25T22:46:27.554Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: dfetch/resources/__init__.py:12-17
Timestamp: 2025-12-25T22:46:27.554Z
Learning: In dfetch, when building standalone binaries with Nuitka, resources are packaged at the top level as "resources" (not "dfetch.resources") to avoid name clashes with the binary executable named "dfetch". The code in dfetch/resources/__init__.py uses `"resources"` string when `__compiled__` is detected, which correctly references the top-level resources folder in the standalone build.
Applied to files:
script/package.py
📚 Learning: 2025-12-25T23:53:09.596Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 861
File: .github/workflows/build.yml:120-125
Timestamp: 2025-12-25T23:53:09.596Z
Learning: When using fpm to create macOS PKG installers, there is a known issue (fpm #1996) where the --prefix value is duplicated during installation. For example, with --prefix=/opt/dfetch, files are installed to /opt/dfetch/opt/dfetch instead of /opt/dfetch. This means PATH entries and references should account for this duplication.
Applied to files:
script/package.py
🧬 Code graph analysis (1)
script/package.py (1)
dfetch/manifest/version.py (1)
Version(6-32)
⏰ 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). (16)
- GitHub Check: run / run (macos-latest, 3.12)
- GitHub Check: run / run (windows-latest, 3.12)
- GitHub Check: run / run (windows-latest, 3.13)
- GitHub Check: run / run (windows-latest, 3.11)
- GitHub Check: run / run (macos-latest, 3.10)
- GitHub Check: run / run (windows-latest, 3.9)
- GitHub Check: run / run (windows-latest, 3.10)
- GitHub Check: run / run (macos-latest, 3.9)
- GitHub Check: run / run (windows-latest, 3.14)
- GitHub Check: build / build (windows-latest)
- GitHub Check: build / build (ubuntu-latest)
- GitHub Check: build / build (macos-latest)
- GitHub Check: run / test-cygwin
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: DevContainer Build & Test
🔇 Additional comments (15)
doc/contributing.rst (1)
111-112: LGTM!Documentation accurately reflects the new automated release workflow where
ci.ymlcreates a draft release and publishing triggers PyPI deployment.script/package.py (2)
49-56: LGTM!The
PLATFORM_NAMEconstant provides clear platform identification for installer naming. The logic correctly handles Linux (default "nix"), macOS ("osx"), and Windows ("win").
72-82: Version usage pattern is appropriate.Using
__version__(with dev suffix) for installer filenames and__digit_only_version__for package metadata (fpm's-vflag) correctly separates human-readable artifact naming from strict semver requirements of package managers..github/workflows/ci.yml (1)
1-17: Well-structured CI orchestration workflow.The centralized orchestration pattern with explicit permissions and trigger conditions is clean. The tag pattern
'[0-9]+.[0-9]+.[0-9]+'correctly matches semantic version tags..github/workflows/run.yml (3)
3-4: Trigger change aligns with centralized orchestration.Changing from push/pull_request to
workflow_callcorrectly integrates this workflow into the CI orchestration defined inci.yml.
60-64: No action needed. Python 3.14 is a stable release (3.14.0 shipped October 7, 2025, with bugfix versions in December 2025), not an alpha/beta version. Including it in the CI matrix is appropriate.Likely an incorrect or invalid review comment.
34-36: Remove unnecessary Zig installation on Windows.Zig is not a dependency of dfetch and is not referenced anywhere in the codebase, configuration files, or git history. The dfetch commands that follow (environment, validate, check, update, report -t sbom) do not require Zig. This installation step should be removed from both the
test-cygwinjob (lines 34-36) and therunjob (lines 104-106).Likely an incorrect or invalid review comment.
.github/workflows/release.yml (2)
51-58: If-condition syntax is now correct.The condition has been fixed to properly evaluate within the expression:
${{ steps.release_info.outputs.tag == 'latest' }}.
66-76: Release deletion now properly scoped to 'latest' tag only.The deletion step is now gated by
if: ${{ steps.release_info.outputs.tag == 'latest' }}, which limits the aggressive deletion behavior to only the rolling "latest" release. This is a reasonable approach for a continuously-updated pre-release tag.script/create_release_notes.py (1)
52-59: Error handling for missing file is now correct.The script now properly exits with code 1 when the changelog file is missing, which will fail the CI workflow appropriately.
.github/workflows/build.yml (5)
4-8: Workflow now acceptsrelease_idinput for release coordination.The change from push/pull_request triggers to
workflow_callwith a requiredrelease_idinput properly integrates this workflow into the centralized CI orchestration.
31-33: Checkout configuration enables version detection.
fetch-depth: 0fetches all history and tags, which is required forsetuptools_scmto determine the version from git tags.persist-credentials: falseis a security best practice.
105-119: Race condition mitigated withpreserve_order: true.Adding
preserve_order: trueforces sequential asset uploads, which helps avoid race conditions when multiple matrix instances upload to the same release. This addresses the concern from the previous review.
174-200: Clean wheel build job.The
build-whljob correctly builds and stores Python wheel distributions. Usingpython -m buildis the modern, recommended approach for building Python packages.
202-224: Wheel upload job follows best practices.The
releasejob properly:
- Gates on
inputs.release_id- Depends on
build-whlto ensure artifacts exist- Uses
preserve_order: truefor sequential uploads- Has appropriate permissions
Note: The
buildjob andreleasejob may still run concurrently (both upload to the same release). Consider whether this could cause issues or if GitHub's release API handles concurrent uploads gracefully.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.