ci: fix build and modernize CI/CD workflows#117
Conversation
AI-Generated Change: - Model: claude-sonnet-4-6 - Intent: fix failing publish_alpha CI — python -m build failed without pyproject.toml - Impact: added pyproject.toml (setuptools build-system), fixed MANIFEST.in to include requirements.txt in sdist, added __version__ to version.py - Verified via: python -m build runs successfully locally Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 29 minutes and 47 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughPackaging was reworked: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
The automated pipeline has reached its destination. 🏁I've aggregated the results of the automated checks for this PR below. 🔍 LintI've finished the analysis you requested. 💡 ✅ ruff: no issues 🔒 Security (pip-audit)Checking for any potential security breaches. 🔓 ✅ No known vulnerabilities found (70 packages scanned). 🔌 Skill Tests (ovoscope)Let's see how the skill performs under the microscope. 🔬 ✅ 2/2 passed ✅ TestSkillLoading — 2/2 🎙️ SkillThe skill check report is now complete. 📝 🎙️ (unknown skill_id) — 13 languages en-US: 7 intents · 5 vocab · 6 dialogs · skill.json Translation coverage — 12 languages (11 complete, 1 incomplete)
🚌 Bus CoverageA deep dive into the skill's communication patterns. 🌊 🏷️ Release PreviewHere's what the next release might look like! 🚀 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
⚖️ License CheckVerifying the SPDX identifiers for correctness. 🆔 ✅ No license violations found (50 packages). License distribution: 12× MIT License, 8× Apache Software License, 8× MIT, 6× Apache-2.0, 2× BSD-3-Clause, 2× ISC License (ISCL), 2× PSF-2.0, 2× Python Software Foundation License, +7 more Full breakdown — 50 packages
Copyright (c) 2022 Phil Ewels Permission is hereby granted, free of charge, to any person obtaining a copy The above copyright notice and this permission notice shall be included in all THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔨 Build TestsThe build process has successfully terminated. 🏁 ✅ All versions pass
📋 Repo HealthA detailed health report for the project. 📝 ✅ All required files present. Latest Version: ✅ 📊 CoverageDiving deep into the code to see what's covered! 🤿 ❌ 40.7% total coverage Per-file coverage (2 files)
Full report: download the Catch you at the next merge! 🌊 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
version.py (1)
7-8: Version string logic is now duplicated betweenversion.pyandsetup.py.
setup.py:get_version()(lines 50–71) constructs the sameMAJOR.MINOR.BUILDa{ALPHA}string by text-parsing this file. The two will silently drift if the format ever changes (e.g., adding a.postsuffix or switchingVERSION_ALPHAto a string). Consider havingsetup.pysource the version from this module so there's a single source of truth.♻️ One option: read `__version__` from version.py in setup.py
# setup.py def get_version(): version_file = os.path.join(os.path.dirname(__file__), 'version.py') ns = {} with open(version_file) as f: exec(f.read(), ns) return ns['__version__']This avoids the brittle line-by-line parser and keeps
version.pyas the single source of truth.Minor behavioral note: the new expression treats
VERSION_ALPHAvia Python truthiness, whilesetup.pyusesint(alpha). Identical for integer literals (current usage), but they would diverge if anyone ever setsVERSION_ALPHA = "0"(truthy as a string, falsy as int). Consolidating to one implementation removes that footgun.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@version.py` around lines 7 - 8, The version string construction is duplicated and can drift: change setup.py to import or exec-load the single source of truth from version.py instead of parsing it; specifically have setup.py’s get_version() obtain __version__ defined in version.py (which builds __version__ from VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA) so both modules use the same logic and truthiness for VERSION_ALPHA (avoid reimplementing the f"{...}" logic in setup.py).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@version.py`:
- Around line 7-8: The version string construction is duplicated and can drift:
change setup.py to import or exec-load the single source of truth from
version.py instead of parsing it; specifically have setup.py’s get_version()
obtain __version__ defined in version.py (which builds __version__ from
VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA) so both modules use
the same logic and truthiness for VERSION_ALPHA (avoid reimplementing the
f"{...}" logic in setup.py).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 535342d8-d673-4bac-91f5-bed050193543
📒 Files selected for processing (3)
MANIFEST.inpyproject.tomlversion.py
- Replaced stub pyproject.toml with complete metadata (name, description, author, license, dependencies, entry points, package-data) - Removed setup.py, requirements.txt, and MANIFEST.in - Dynamic version reads from ovos_skill_volume.version.__version__ - Entry point: ovos.plugin.skill -> ovos-skill-volume.openvoiceos = ovos_skill_volume:VolumeSkill - Build verified: python -m build produces correct wheel at 0.1.21a2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Change dynamic version attr from `ovos_skill_volume.version.__version__` to `version.__version__` so setuptools can resolve it at build time without needing the package pre-installed - Replace `packages.find` (which cannot discover flat-layout packages) with explicit `packages = ["ovos_skill_volume"]` - Fix package-dir mapping from `""` to `"."` (equivalent, but clearer) - Add `[project.optional-dependencies] test` extras so CI `.[test]` install works Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pyproject.toml (1)
27-31: Drop the duplicatedovos-workshopfrom thetestextra.
ovos-workshop>=8.0.0,<9.0.0is already a hard runtime dependency (line 22), so listing it again in thetestextra is redundant and risks the two pins drifting out of sync over time. Test-only utilities (e.g.pytest) are sufficient here.♻️ Proposed change
[project.optional-dependencies] test = [ "pytest", - "ovos-workshop>=8.0.0,<9.0.0", ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 27 - 31, Remove the duplicated runtime dependency from the test extra: delete "ovos-workshop>=8.0.0,<9.0.0" from the [project.optional-dependencies] test list so only test-only utilities like "pytest" remain; keep the hard runtime pin (the existing ovos-workshop entry) in the main project dependencies and ensure the test extra only includes test-specific packages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 2: Update pyproject.toml to require setuptools>=62.3.0 instead of >=42 so
the recursive glob patterns in package-data (locale/**, ui/**, vocab/**,
dialog/**, regex/**, skill/**) are honored during sdist/wheel builds; also
remove the duplicate ovos-workshop>=8.0.0,<9.0.0 entry from the "test" extra
(keep it in the main dependencies) so it’s not redundantly listed. Use the
existing "requires" key to bump the setuptools version and edit the extras[test]
block to delete the ovos-workshop line.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 27-31: Remove the duplicated runtime dependency from the test
extra: delete "ovos-workshop>=8.0.0,<9.0.0" from the
[project.optional-dependencies] test list so only test-only utilities like
"pytest" remain; keep the hard runtime pin (the existing ovos-workshop entry) in
the main project dependencies and ensure the test extra only includes
test-specific packages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| @@ -0,0 +1,57 @@ | |||
| [build-system] | |||
| requires = ["setuptools>=42", "wheel"] | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the minimum setuptools version that supports recursive ** glob patterns in package-data / package_data?
💡 Result:
The minimum setuptools version that supports recursive glob patterns (using ) in package_data / package_data is 62.3.0.
Citations:
- 1: Allow recursive globs for package_data pypa/setuptools#3309
- 2: https://stackoverflow.com/questions/27664504/how-to-add-package-data-recursively-in-python-setup-py
- 3: Simplify package_data handling in setup.py pytorch/pytorch#148256
- 4: https://setuptools.pypa.io/en/stable/history.html
- 5: https://stackoverflow.com/questions/72060314/including-a-data-folder-in-a-python-package-using-setup-cfg
🏁 Script executed:
cat pyproject.tomlRepository: OpenVoiceOS/ovos-skill-volume
Length of output: 1387
Bump setuptools minimum to >=62.3.0 — recursive ** globs in package-data require this version.
The package-data entries (locale/**/*, ui/**/*, vocab/**/*, dialog/**/*, regex/**/*, skill/**/*) use recursive glob patterns that were added to setuptools in version 62.3.0. With setuptools>=42, older build environments that resolve setuptools <62.3.0 will silently ignore these patterns, causing locale, ui, vocab, dialog, regex, and skill assets to be missing from both sdist and wheel — re-introducing the exact FileNotFoundError packaging issues this PR aims to fix.
Additionally, ovos-workshop>=8.0.0,<9.0.0 appears in both dependencies (line 20) and the test extra (line 28), which is redundant since it is already available at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` at line 2, Update pyproject.toml to require
setuptools>=62.3.0 instead of >=42 so the recursive glob patterns in
package-data (locale/**, ui/**, vocab/**, dialog/**, regex/**, skill/**) are
honored during sdist/wheel builds; also remove the duplicate
ovos-workshop>=8.0.0,<9.0.0 entry from the "test" extra (keep it in the main
dependencies) so it’s not redundantly listed. Use the existing "requires" key to
bump the setuptools version and edit the extras[test] block to delete the
ovos-workshop line.
- Add ovos-plugin-manager and ovos-utils to [test] extras in pyproject.toml so CI install_extras=test pulls in all test dependencies - Rewrite test_skill_loading.py: remove stale adapt/mycroft/skill_ovos_volume imports that caused ModuleNotFoundError; use current ovos_skill_volume module name and modern ovos-workshop API - Fix ovoscope.yml test_path from non-existent test/end2end/ to test/unittests/ so pytest collects tests instead of exiting with code 4 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
pyproject.tomlwith[build-system]table sopython -m buildworks (fixes the failingpublish_alpha / publish_pypijob at the "Build Distribution Packages" step)MANIFEST.into includerequirements.txtin the sdist (was causingFileNotFoundErrorwhen building from sdist)__version__toversion.pyfor standard Python version exportRoot Cause
The CI run at https://github.com/OpenVoiceOS/ovos-skill-volume/actions/runs/24203261408/job/70655143122 failed because
python -m buildwas invoked by thepublish-alphareusable workflow, but there was nopyproject.toml. Modernbuildrequires a[build-system]table. Without it, the build falls back to a legacy sdist flow that then fails whenrequirements.txtis not present in the sdist.Existing Workflows
All existing workflows already use
OpenVoiceOS/gh-automations@devrefs — no ref migration needed. The full standard OVOS skill workflow set is already present:build-tests.yml,release_workflow.yml,publish_stable.yml,coverage.yml,lint.yml,ovoscope.yml,skill-check.yml,pip_audit.yml,license_check.yml,repo-health.yml,release-preview.yml,conventional-label.ymlTest plan
python -m buildcompletes without error after mergingrelease_workflow.ymlviaworkflow_dispatchto confirmpublish_alpha / publish_pypisucceedsPYPI_TOKENandMATRIX_TOKENare set as org-level secretsReminder
Ensure these secrets are set at org or repo level before the publish workflow runs:
PYPI_TOKEN— PyPI API tokenMATRIX_TOKEN— Matrix access token🤖 Generated with Claude Code
Summary by CodeRabbit
Build & Packaging
Chores