Conversation
There was a problem hiding this comment.
Pull request overview
This pull request prepares the kv-dict project for public release by migrating from an internal GitHub Enterprise instance to public GitHub, enabling the release workflow, and updating installation instructions to use package managers instead of git URLs.
Changes:
- Updated all GitHub URLs in README.md from internal (
github.lightsource.ca) to public GitHub (github.com/Canadian-Light-Source) - Changed installation instructions from git-based to PyPI package manager installations (
pip install kv-dict,uv add kv-dict,pixi add --pypi kv-dict) - Enabled the release workflow in ci.yml to automatically create GitHub releases when tags are pushed
- Added new
_release.ymlworkflow file to handle artifact downloading and GitHub release creation with prerelease detection
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Updated GitHub URLs from internal to public, changed installation instructions from git URLs to package manager commands |
| .github/workflows/ci.yml | Uncommented and enabled the release job to run on tag pushes |
| .github/workflows/_release.yml | New reusable workflow for creating GitHub releases with artifacts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/_release.yml
Outdated
| prerelease: | ||
| ${{ contains(github.ref_name, 'a') || contains(github.ref_name, 'b') | ||
| || contains(github.ref_name, 'rc') }} | ||
| files: "*" | ||
| generate_release_notes: true |
There was a problem hiding this comment.
The prerelease detection logic is too simplistic and could produce false positives. It checks if the tag name contains 'a', 'b', or 'rc' anywhere in the string, which means tags like "feature-branch" or "stable" would incorrectly be marked as prereleases. Consider using a more robust pattern such as checking for 'a[0-9]', 'b[0-9]', or 'rc[0-9]' which aligns with PEP 440 prerelease versioning (e.g., 1.0.0a1, 1.0.0b2, 1.0.0rc1).
| prerelease: | |
| ${{ contains(github.ref_name, 'a') || contains(github.ref_name, 'b') | |
| || contains(github.ref_name, 'rc') }} | |
| files: "*" | |
| generate_release_notes: true | |
| prerelease: ${{ contains(github.ref_name, 'a0') || contains(github.ref_name, 'a1') || contains(github.ref_name, 'a2') || contains(github.ref_name, 'a3') || contains(github.ref_name, 'a4') || contains(github.ref_name, 'a5') || contains(github.ref_name, 'a6') || contains(github.ref_name, 'a7') || contains(github.ref_name, 'a8') || contains(github.ref_name, 'a9') || contains(github.ref_name, 'b0') || contains(github.ref_name, 'b1') || contains(github.ref_name, 'b2') || contains(github.ref_name, 'b3') || contains(github.ref_name, 'b4') || contains(github.ref_name, 'b5') || contains(github.ref_name, 'b6') || contains(github.ref_name, 'b7') || contains(github.ref_name, 'b8') || contains(github.ref_name, 'b9') || contains(github.ref_name, 'rc0') || contains(github.ref_name, 'rc1') || contains(github.ref_name, 'rc2') || contains(github.ref_name, 'rc3') || contains(github.ref_name, 'rc4') || contains(github.ref_name, 'rc5') || contains(github.ref_name, 'rc6') || contains(github.ref_name, 'rc7') || contains(github.ref_name, 'rc8') || contains(github.ref_name, 'rc9') }} | |
| files: "*" | |
| generate_release_notes: true | |
| files: "*" | |
| generate_release_notes: true |
.github/workflows/_release.yml
Outdated
| # We pin to the SHA, not the tag, for security reasons. | ||
| # https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions | ||
| uses: softprops/action-gh-release@v2 |
There was a problem hiding this comment.
The comment states "We pin to the SHA, not the tag, for security reasons," but the action is actually using the tag '@v2' instead of a SHA. For consistency with the stated security best practice and the comment, consider pinning to a specific commit SHA (e.g., uses: softprops/action-gh-release@9d7c94c).
match upload version Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
format changes for Note and Warning blocks to render correctly.
re-add codeql
add release workflow, based on DLS