chore: align with CI workflows of other interface packages #1
Conversation
📝 WalkthroughWalkthroughThe pull request introduces significant changes to the GitHub Actions workflows for the project. A new Changes
Sequence DiagramsequenceDiagram
participant PR as Pull Request
participant Workflow as Conventional PRs Workflow
participant Validator as Semantic PR Validator
PR->>Workflow: Trigger on PR events
Workflow->>Validator: Check PR title
alt Valid Semantic Title
Validator-->>Workflow: Validation Passed
else Invalid Title
Validator-->>Workflow: Validation Failed
Workflow-->>PR: Request Title Correction
end
sequenceDiagram
participant Repo as Repository
participant ReleasePlease as Release Please Workflow
participant PyPI as PyPI Registry
participant GitHub as GitHub Releases
Repo->>ReleasePlease: Push to main branch
ReleasePlease->>ReleasePlease: Create Release
ReleasePlease->>PyPI: Publish Package
ReleasePlease->>GitHub: Create GitHub Release
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/test.yml (2)
7-8: Remove unnecessarybranches-ignoreconfiguration.The empty
branches-ignorearray doesn't serve any purpose and can be removed.pull_request: - branches-ignore: []🧰 Tools
🪛 actionlint (1.7.4)
8-8: "branches-ignore" section should not be empty
(syntax-check)
91-91: Consider using a more flexible test discovery pattern.The current pytest command uses a hardcoded test file path. Consider using a more flexible pattern to automatically discover all test files:
- run: poetry run coverage run -m pytest tests/tests.py + run: poetry run coverage run -m pytest tests/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/conventional-prs.yml(1 hunks).github/workflows/release-please.yml(1 hunks).github/workflows/release.yml(0 hunks).github/workflows/test.yml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/release.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-please.yml
25-25: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
27-27: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
37-37: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test.yml
8-8: "branches-ignore" section should not be empty
(syntax-check)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
17-17: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
27-27: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
42-42: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
44-44: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
54-54: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
68-68: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
70-70: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
80-80: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/conventional-prs.yml (1)
1-16: LGTM! Good security practices withpull_request_target.The workflow is well-configured for semantic PR title validation. Using
pull_request_targetis appropriate here since the workflow only uses a trusted action and doesn't check out or run PR code.
.github/workflows/release-please.yml
Outdated
| needs: release-please | ||
| if: ${{ needs.release-please.outputs.release_created }} | ||
| steps: | ||
| - uses: actions/checkout@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update GitHub Actions to their latest versions.
Several actions are using outdated versions:
actions/checkout@v3->actions/checkout@v4actions/setup-python@v2->actions/setup-python@v5actions/setup-python@v4->actions/setup-python@v5
Also applies to: 27-27, 37-37
🧰 Tools
🪛 actionlint (1.7.4)
25-25: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/release-please.yml
Outdated
| - uses: actions/setup-python@v2 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate Python setup steps.
The Python setup step is duplicated. The first setup can be removed since the second one includes caching.
- - uses: actions/setup-python@v2
- with:
- python-version: "3.11"
-
- name: Install poetry
run: pip install poetry
- name: Determine dependencies
run: poetry lock
- uses: actions/setup-python@v4
with:
python-version: "3.11"
cache: poetry📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/setup-python@v2 | |
| with: | |
| python-version: "3.11" | |
| - name: Install poetry | |
| run: pip install poetry | |
| - name: Determine dependencies | |
| run: poetry lock | |
| - uses: actions/setup-python@v4 | |
| with: | |
| python-version: "3.11" | |
| cache: poetry | |
| - name: Install poetry | |
| run: pip install poetry | |
| - name: Determine dependencies | |
| run: poetry lock | |
| - uses: actions/setup-python@v4 | |
| with: | |
| python-version: "3.11" | |
| cache: poetry |
🧰 Tools
🪛 actionlint (1.7.4)
27-27: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
37-37: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/test.yml
Outdated
| formatting: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check out the code | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
||
| - name: Install Dependencies using Poetry | ||
| run: poetry install | ||
|
|
||
| - name: Check formatting | ||
| run: poetry run black --check . | ||
|
|
||
| linting: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check out the code | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
||
| - name: Install Dependencies using Poetry | ||
| run: poetry install | ||
|
|
||
| - name: Check code | ||
| run: poetry run flake8 | ||
|
|
||
| testing: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install poetry | ||
| run: pip install poetry | ||
|
|
||
| - name: Determine dependencies | ||
| run: poetry lock | ||
|
|
||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: poetry | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| pip install connection-pool # because it is incompatible with poetry | ||
| poetry install | ||
|
|
||
| - name: Run pytest | ||
| run: poetry run coverage run -m pytest tests/tests.py | ||
|
|
||
| - name: Run Coverage | ||
| run: poetry run coverage report -m |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor workflow to reduce duplication using reusable workflows.
Each job (formatting, linting, testing) has identical setup steps. Consider extracting these common steps into a reusable workflow to improve maintainability.
Here's how you could refactor this:
- Create a new file
.github/workflows/setup-poetry.yml:
name: Setup Poetry
on:
workflow_call:
jobs:
setup:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: "3.11"
cache: poetry
- name: Install poetry
run: pip install poetry
- name: Determine dependencies
run: poetry lock
- name: Install Dependencies
run: poetry install- Update test.yml to use the reusable workflow:
name: CI
on:
push:
branches:
- main
pull_request:
jobs:
setup:
uses: ./.github/workflows/setup-poetry.yml
formatting:
needs: setup
runs-on: ubuntu-latest
steps:
- name: Check formatting
run: poetry run black --check .
linting:
needs: setup
runs-on: ubuntu-latest
steps:
- name: Check code
run: poetry run flake8
testing:
needs: setup
runs-on: ubuntu-latest
steps:
- name: Install connection-pool
run: pip install connection-pool
- name: Run pytest
run: poetry run coverage run -m pytest tests/tests.py
- name: Run Coverage
run: poetry run coverage report -m🧰 Tools
🪛 actionlint (1.7.4)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
17-17: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
27-27: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
42-42: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
44-44: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
54-54: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
68-68: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
70-70: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
80-80: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
.github/workflows/test.yml
Outdated
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check out the code | ||
| uses: actions/checkout@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update GitHub Actions to their latest versions.
Several actions are using outdated versions:
actions/checkout@v3->actions/checkout@v4actions/setup-python@v4->actions/setup-python@v5
Also applies to: 42-42, 68-68, 17-17, 27-27, 44-44, 54-54, 70-70, 80-80
🧰 Tools
🪛 actionlint (1.7.4)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/snakemake_interface_logger_plugins/registry/__init__.py (1)
Line range hint
8-8: Remove unused import.The
Listtype fromtypingis imported but never used.-from typing import List, Mapping +from typing import Mapping
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
11-54: 🛠️ Refactor suggestionRefactor workflow to reduce duplication.
The jobs have identical setup steps. Consider extracting these common steps into a reusable workflow.
This is a duplicate of a previous review comment. The suggestion to create a reusable workflow remains valid.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
🧹 Nitpick comments (2)
.github/workflows/release-please.yml (1)
34-39: Add error handling for build steps.Consider adding error handling and validation steps:
- Verify the build artifacts after creation
- Add timeouts to prevent hanging builds
- name: Install the project run: uv sync --all-extras --dev + timeout-minutes: 10 - name: Build source and wheel distribution run: | uv build + # Verify the build artifacts + ls dist/*.whl dist/*.tar.gz + timeout-minutes: 5tests/tests.py (1)
55-92: Consider adding more test scenarios.While the test registry implementation is good, consider adding more test cases:
- Test with invalid log levels
- Test handler creation with various combinations of boolean parameters
Would you like me to generate additional test cases to improve coverage?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pyproject.tomlis excluded by!pyproject.tomluv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/release-please.yml(1 hunks).github/workflows/test.yml(1 hunks)src/snakemake_interface_logger_plugins/registry/__init__.py(2 hunks)tests/tests.py(1 hunks)version.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- version.txt
🧰 Additional context used
📓 Path-based instructions (2)
src/snakemake_interface_logger_plugins/registry/__init__.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
tests/tests.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
🪛 actionlint (1.7.4)
.github/workflows/test.yml
8-8: "branches-ignore" section should not be empty
(syntax-check)
.github/workflows/release-please.yml
24-24: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
42-42: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
56-56: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
.github/workflows/release-please.yml
[error] 28-28: trailing spaces
(trailing-spaces)
🪛 GitHub Actions: CI
src/snakemake_interface_logger_plugins/registry/__init__.py
[error] 9-9: typing.List imported but unused. Remove unused import: typing.List
🔇 Additional comments (6)
.github/workflows/release-please.yml (2)
24-24: Update actions/checkout to v4.The checkout action is using an outdated version.
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4🧰 Tools
🪛 actionlint (1.7.4)
24-24: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
46-62: LGTM! Secure PyPI publishing configuration.The PyPI publishing job is well configured with:
- Trusted publishing using OpenID Connect
- Proper permissions setup
- Secure artifact handling
🧰 Tools
🪛 actionlint (1.7.4)
56-56: the runner of "actions/download-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
src/snakemake_interface_logger_plugins/registry/__init__.py (2)
23-23: LGTM! Architectural change fromLoggerPluginBasetoLogHandlerBase.The change aligns with the PR's objective to update the interface package.
51-51: LGTM! Updated expected attribute type toLogHandlerBase.The change is consistent with the import change and maintains the architectural consistency.
tests/tests.py (2)
16-28: LGTM! Well-structured mock settings class.The
MockSettingsclass provides a good example of how to implement logger plugin settings with proper metadata.
30-53: LGTM! Comprehensive mock plugin implementation.The
MockPluginclass correctly implements the required interface with proper default settings handling.
| - uses: GoogleCloudPlatform/release-please-action@v3 | ||
| id: release | ||
| with: | ||
| release-type: simple |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update release-type for Python package.
The release-type should be set to "python" instead of "simple" for Python packages to ensure proper version management and changelog generation.
- release-type: simple
+ release-type: python📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| release-type: simple | |
| release-type: python |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
There was a problem hiding this comment.
Remove trailing spaces.
There are trailing spaces on lines 15, 29, and 43.
- - uses: actions/checkout@v4
-
+ - uses: actions/checkout@v4Also applies to: 29-29, 43-43
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 15-15: trailing spaces
(trailing-spaces)
| branches: | ||
| - main | ||
| pull_request: | ||
| branches-ignore: [] |
There was a problem hiding this comment.
Remove empty branches-ignore section.
The empty branches-ignore section is unnecessary and can be removed.
pull_request:
- branches-ignore: []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| branches-ignore: [] | |
| pull_request: |
🧰 Tools
🪛 actionlint (1.7.4)
8-8: "branches-ignore" section should not be empty
(syntax-check)
|
Hi @johanneskoester, I updated the workflows to use ruff for formatting and linting. Had some trouble with pylyzer so gave up on that for now. Also stuck with uv over poetry. I don't think I realized the other plugins used poetry - happy to migrate this to poetry, though I do prefer uv. I'm unfamiliar with release-please and how exactly it works and the workflow, but it seems handy. |
I have simply copied over the CI workflows of the other interface repos. I did not know about pypi trusted publishing though. Feel free to modernize this accordingly by integrating the pypi publishing you had in your previous version. Using release please is very handy though.
Summary by CodeRabbit
New Features
Workflow Changes
Version Update
0.1.1