ci: add maintainer baseline checks - #20
Conversation
|
Warning Review limit reached
More reviews will be available in 33 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR modernizes release management and packaging infrastructure by replacing release-drafter with Release Please automation, establishes CI/CD pipelines for testing and documentation builds, refines package metadata, adds a changelog, updates user-facing documentation, and implements package validation tests. ChangesRelease Automation and Packaging
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/ci.yml (2)
17-18: ⚖️ Poor tradeoffConsider pinning actions to commit SHAs for supply chain security.
Actions are currently pinned to major version tags (
@v4,@v5), which can float to new minor/patch releases. For stronger supply chain guarantees, you can pin to immutable commit SHAs:- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0This prevents potential supply chain attacks where a tag is moved to malicious code, though it increases maintenance burden. Many projects accept the tag-based approach as a reasonable tradeoff. As per static analysis hints, actions are not pinned to a hash as required by blanket policy.
Also applies to: 40-41
🤖 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 17 - 18, Replace floating tag references for GitHub Actions with immutable commit SHAs: locate the uses entries "actions/checkout@v4" and "actions/setup-python@v5" and update them to their corresponding commit SHA pins (e.g., the commit SHAs for the intended versions) so the workflow uses exact immutable revisions; apply the same change to the other occurrences of these actions noted in the diff (the second "actions/checkout" and "actions/setup-python" uses) to ensure all action usages are pinned consistently.
1-51: ⚡ Quick winConsider scoping workflow permissions to read-only.
The workflow inherits default repository permissions, which may include write access. Since this CI workflow only validates code and doesn't push changes or create comments, you can harden security by adding an explicit read-only permissions block at the workflow level:
name: CI on: pull_request: push: branches: [main] permissions: contents: read jobs: test: # ...This follows the principle of least privilege and addresses the
excessive-permissionsfinding from static analysis. As per static analysis hints, default permissions are used due to no explicit permissions block.🤖 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 1 - 51, Add an explicit workflow-level permissions block to restrict repository access to read-only by adding a top-level "permissions" mapping with "contents: read" in the CI workflow (i.e., alongside the existing name/on keys), and verify no job/step in the workflow (such as publishing or commenting steps) requires write permissions; if any do, either remove/adjust them or scope permissions more narrowly for those jobs instead of allowing default write access.tests/test_package_smoke.py (1)
27-28: ⚡ Quick winTest name suggests comparison but only checks existence.
The function name
test_distribution_metadata_matches_importable_packageimplies that some matching or comparison is performed, but the test only verifies that metadata is retrievable. Consider either:
- Renaming to
test_distribution_metadata_is_accessible()to reflect actual behavior, or- Adding a version comparison to match the implied behavior.
Option 1: Rename to clarify intent
-def test_distribution_metadata_matches_importable_package(): +def test_distribution_metadata_is_accessible(): assert importlib.metadata.version('unifi-controller-api')Option 2: Add version comparison to match name
def test_distribution_metadata_matches_importable_package(): - assert importlib.metadata.version('unifi-controller-api') + version = importlib.metadata.version('unifi-controller-api') + assert version == '0.3.2'🤖 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 `@tests/test_package_smoke.py` around lines 27 - 28, Rename the test function currently named test_distribution_metadata_matches_importable_package to test_distribution_metadata_is_accessible and update its definition accordingly so it reflects the actual assertion (i.e., keep the body assert importlib.metadata.version('unifi-controller-api')); locate the function by the symbol name test_distribution_metadata_matches_importable_package in tests/test_package_smoke.py and change only the function name to the new clearer name.
🤖 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/release-please.yml:
- Line 16: Replace the mutable tag used for the GitHub Action by resolving the
current commit SHA that the ref googleapis/release-please-action@v4 points to
and update the workflow line that currently reads
"googleapis/release-please-action@v4" to use the exact immutable commit SHA
(e.g., googleapis/release-please-action@<commit-sha>); ensure you copy the full
40-character SHA for the release-please-action repository and commit, commit the
updated .github/workflows/release-please.yml, and verify the workflow runs
against that SHA.
In `@docs/api/overview.rst`:
- Around line 53-54: The example imports and calls export_to_csv but the public
helper is named export_csv; update the snippet so both the import and usage
reference export_csv (replace export_to_csv with export_csv in the import
statement and the function call) to ensure the example matches the actual API
(look for occurrences of export_to_csv in the example and change them to
export_csv).
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 17-18: Replace floating tag references for GitHub Actions with
immutable commit SHAs: locate the uses entries "actions/checkout@v4" and
"actions/setup-python@v5" and update them to their corresponding commit SHA pins
(e.g., the commit SHAs for the intended versions) so the workflow uses exact
immutable revisions; apply the same change to the other occurrences of these
actions noted in the diff (the second "actions/checkout" and
"actions/setup-python" uses) to ensure all action usages are pinned
consistently.
- Around line 1-51: Add an explicit workflow-level permissions block to restrict
repository access to read-only by adding a top-level "permissions" mapping with
"contents: read" in the CI workflow (i.e., alongside the existing name/on keys),
and verify no job/step in the workflow (such as publishing or commenting steps)
requires write permissions; if any do, either remove/adjust them or scope
permissions more narrowly for those jobs instead of allowing default write
access.
In `@tests/test_package_smoke.py`:
- Around line 27-28: Rename the test function currently named
test_distribution_metadata_matches_importable_package to
test_distribution_metadata_is_accessible and update its definition accordingly
so it reflects the actual assertion (i.e., keep the body assert
importlib.metadata.version('unifi-controller-api')); locate the function by the
symbol name test_distribution_metadata_matches_importable_package in
tests/test_package_smoke.py and change only the function name to the new clearer
name.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b9c22d6-e428-4e60-808f-124e0e0085ed
📒 Files selected for processing (18)
.github/release-drafter.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/release-drafter.yml.github/workflows/release-please.yml.release-please-manifest.jsonCHANGELOG.mdREADME.mddocs/api/export.rstdocs/api/overview.rstdocs/api/utilities.rstdocs/changelog.rstdocs/contributing.rstdocs/examples.rstdocs/index.rstpyproject.tomlrelease-please-config.jsontests/test_package_smoke.py
💤 Files with no reviewable changes (2)
- .github/workflows/release-drafter.yml
- .github/release-drafter.yml
Summary
Adds a basic maintainer safety baseline before making user-visible API behavior changes:
-Wraw=FalseCHANGELOG.mdWhy
The project has real downstream usage, but currently has no PR CI gate, no tests, and release-time docs failures. This PR establishes the minimum safety net needed before addressing higher-risk API behavior fixes.
Validation
ruff check .✅pytest -q✅3 passedpython -m build✅twine check dist/*✅sphinx-build -E -W -b html docs docs/_build/html✅Follow-ups intentionally left out
Summary by CodeRabbit
Chores
Documentation