ci: migrate CI configuration from CircleCI to GitHub Actions - #22
Open
kilo-code-bot[bot] wants to merge 7 commits into
Open
ci: migrate CI configuration from CircleCI to GitHub Actions#22kilo-code-bot[bot] wants to merge 7 commits into
kilo-code-bot[bot] wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces CircleCI and ChangesPackaging and CI migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 22-23: Add a real automated test suite with a declared test
runner, then update the CI workflow’s “Run tests” step to execute that runner
and discover the test files. Keep the existing python complex_colormap/cplot.py
invocation as a separate smoke-test step rather than using it as test coverage.
- Around line 8-12: Update the test job in the workflow to set job-level
permissions to contents: read and configure the actions/checkout@v4 step with
persist-credentials disabled. Keep the existing checkout and test job structure
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e205f90-37bf-48b2-8b4a-5770eea888a7
📒 Files selected for processing (2)
.circleci/config.yml.github/workflows/ci.yml
💤 Files with no reviewable changes (1)
- .circleci/config.yml
Migrate the project packaging from setup.py and requirements.txt to pyproject.toml. Update Makefile to use the `build` module instead of `setup.py` and update CI workflows to install the package in editable mode.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 20-23: Update the existing .PHONY declaration in the Makefile to
include both publish and clean, ensuring these targets always execute their
recipes even when files with those names exist.
- Around line 13-16: Update the release tooling configuration associated with
the Makefile upload target to declare the separate Python build package
alongside hatchling, and ensure that package is installed before invoking make
upload. Keep the existing upload sequence and build command unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cc38b2ab-eb80-459a-980e-4dfe826aec95
📒 Files selected for processing (6)
.github/workflows/ci.ymlMakefilecomplex_colormap/__about__.pypyproject.tomlrequirements.txtsetup.py
💤 Files with no reviewable changes (3)
- requirements.txt
- setup.py
- complex_colormap/about.py
- Add generated C_lut.npy lookup table (required at import time) - Include C_lut.npy in the built wheel - Add pytest test suite covering both colormaps - Run pytest in CI with a separate cplot.py smoke test - Harden workflow: contents: read permission, persist-credentials: false - Declare build frontend and PHONY targets in Makefile
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
22-27: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate a built distribution in CI.
Line 24 installs the source tree in editable mode. This does not prove that the wheel contains
C_lut.npy. Add a regular wheel install or an explicit wheel-content check, and run that check outside the repository source path. Pip documents editable installs as development installs whose behavior can differ from regular distribution installs. (pip.pypa.io)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 22 - 27, Update the CI steps around “Install dependencies” and “Run tests” to build and install the regular wheel instead of relying only on pip install -e, then execute the tests from outside the repository source path so the packaged wheel’s C_lut.npy is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_cplot.py`:
- Around line 34-41: Resolve the zero-input contract consistently between
test_colormap_black_for_zero, max_chroma_colormap, and the cplot documentation:
either preserve the lookup-table value by renaming the test and updating the
documentation, or make max_chroma_colormap return black at zero and change the
expected value to (0.0, 0.0, 0.0). Ensure the test name, implementation
behavior, and documentation agree.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 22-27: Update the CI steps around “Install dependencies” and “Run
tests” to build and install the regular wheel instead of relying only on pip
install -e, then execute the tests from outside the repository source path so
the packaged wheel’s C_lut.npy is validated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ae9a41b5-2daa-4646-a686-c10661e70d3b
📒 Files selected for processing (5)
.github/workflows/ci.ymlMakefilecomplex_colormap/C_lut.npypyproject.tomltests/test_cplot.py
The max_chroma_colormap zero-input behavior is a library contract question (docstring prose vs. example disagree), not a CI concern. Leave it for a separate change.
bump requires-python to >=3.10 and add classifiers for 3.10 through 3.13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrate CI from CircleCI to GitHub Actions.
Fixes the CircleCI checkout failure (
Permission denied (publickey)):actions/checkout@v4authenticates over HTTPS with the built-inGITHUB_TOKEN, so no SSH key is required..github/workflows/ci.yml(checkout, Python setup, install requirements, run tests).circleci/config.ymlSummary by CodeRabbit
Chores
Tests