fix: resolve LICENSE conflict, upgrade to PyPI Trusted Publishing, add CHANGELOG#4
Conversation
Agent-Logs-Url: https://github.com/samdporter/SIRF-SIMIND-Connection/sessions/b4b30543-7196-4bba-adb1-9df2d14f7904 Co-authored-by: samdporter <92305641+samdporter@users.noreply.github.com>
Reviewer's GuideThis PR resolves PyPI publication blockers by fixing the corrupted LICENSE, migrating the release workflow to PyPI Trusted Publishing, and adding a formal changelog linked from pyproject.toml. Sequence diagram for PyPI Trusted Publishing workflowsequenceDiagram
actor Maintainer
participant GitHubRepo
participant GitHubActions
participant GitHubOIDC
participant PyPI
Maintainer->>GitHubRepo: Create GitHub release
GitHubRepo-->>GitHubActions: Trigger publish workflow
GitHubActions->>GitHubActions: Build distribution (python -m build)
GitHubActions->>GitHubActions: Run twine check dist/*
GitHubActions->>GitHubOIDC: Request id-token (permissions id-token: write)
GitHubOIDC-->>GitHubActions: Signed OIDC id-token
GitHubActions->>PyPI: Publish via pypa/gh-action-pypi-publish@release/v1 using id-token
PyPI-->>GitHubActions: Validate trusted publisher and accept upload
GitHubActions-->>Maintainer: Publish job completes
Flow diagram for release-to-PyPI pipeline with Trusted Publishingflowchart LR
A[Maintainer creates GitHub release] --> B[GitHub Actions triggers publish job]
B --> C[Set up Python and dependencies]
C --> D[Build package with python -m build]
D --> E[Run twine check dist/*]
E --> F[Request OIDC id-token
permissions id-token: write]
F --> G[Call pypa/gh-action-pypi-publish@release/v1
with id-token]
G --> H[PyPI validates trusted publisher
and uploads py-smc]
H --> I[Release available on PyPI]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/publish.yml" line_range="10-11" />
<code_context>
jobs:
publish:
runs-on: ubuntu-latest
+ permissions:
+ id-token: write
steps:
- uses: actions/checkout@v4
</code_context>
<issue_to_address>
**issue (bug_risk):** Setting only `id-token: write` will revoke other default permissions and likely break `actions/checkout`.
Defining `permissions` overrides all defaults, so anything not listed is set to `none` (including `contents: read`), which `actions/checkout@v4` needs.
To keep checkout working while enabling OIDC for PyPI, you can set:
```yaml
jobs:
publish:
permissions:
contents: read
id-token: write
```
so the workflow still has read access to the repo while granting the required ID token permission.
</issue_to_address>
### Comment 2
<location path="CHANGELOG.md" line_range="17" />
<code_context>
+- Support for SIMIND `.atn` attenuation map data files.
+- Helper utilities for configuring and running SIMIND Monte Carlo simulations from Python.
+- Comprehensive test suite using `pytest`.
+- Documentation hosted on ReadTheDocs.
+- PyPI packaging (`py-smc`) with optional `dev` and `examples` dependency groups.
</code_context>
<issue_to_address>
**nitpick (typo):** Consider using the official service name "Read the Docs" instead of "ReadTheDocs".
The service’s official branding uses spaces: “Read the Docs.” Please update this bullet accordingly, e.g. “Documentation hosted on Read the Docs.”
```suggestion
- Documentation hosted on Read the Docs.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| permissions: | ||
| id-token: write |
There was a problem hiding this comment.
issue (bug_risk): Setting only id-token: write will revoke other default permissions and likely break actions/checkout.
Defining permissions overrides all defaults, so anything not listed is set to none (including contents: read), which actions/checkout@v4 needs.
To keep checkout working while enabling OIDC for PyPI, you can set:
jobs:
publish:
permissions:
contents: read
id-token: writeso the workflow still has read access to the repo while granting the required ID token permission.
| - Support for SIMIND `.atn` attenuation map data files. | ||
| - Helper utilities for configuring and running SIMIND Monte Carlo simulations from Python. | ||
| - Comprehensive test suite using `pytest`. | ||
| - Documentation hosted on ReadTheDocs. |
There was a problem hiding this comment.
nitpick (typo): Consider using the official service name "Read the Docs" instead of "ReadTheDocs".
The service’s official branding uses spaces: “Read the Docs.” Please update this bullet accordingly, e.g. “Documentation hosted on Read the Docs.”
| - Documentation hosted on ReadTheDocs. | |
| - Documentation hosted on Read the Docs. |
There was a problem hiding this comment.
Pull request overview
This PR aims to remove remaining blockers for publishing py-smc to PyPI by cleaning up licensing artifacts, switching the release workflow to PyPI Trusted Publishing (OIDC), and adding a project changelog reference in package metadata.
Changes:
- Add
CHANGELOG.mdand link it frompyproject.toml. - Update the GitHub Actions release workflow to publish via PyPI Trusted Publishing (OIDC) instead of an API token.
- Resolve merge-conflict corruption in
LICENSE(but see review comments about license text integrity).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyproject.toml |
Adds a Changelog URL under [project.urls]. |
LICENSE |
Removes conflict markers/duplicate content; currently contains non-canonical Apache 2.0 text. |
CHANGELOG.md |
Introduces a Keep a Changelog–style changelog with a 0.5.0 entry. |
.github/workflows/publish.yml |
Switches publish step to Trusted Publishing and adds OIDC permissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Work" shall mean the work of authorship made available under | ||
| the License, as indicated by a copyright notice that is included in | ||
| or attached to the work (an example is provided in the Appendix below). | ||
|
|
||
| "Derivative Works" shall mean any work, whether in Source or Object |
| jobs: | ||
| publish: | ||
| runs-on: ubuntu-latest | ||
| permissions: |
Three blockers preventing clean PyPI publication: a corrupt
LICENSEfile with unresolved merge conflicts, an API-token-based publish workflow, and no changelog.Changes
LICENSE— Stripped merge conflict markers (<<<<<<< HEAD/=======/>>>>>>>); replaced with a single clean Apache 2.0 text underCopyright 2024 Sam Porter, Efstathios Varzakis..github/workflows/publish.yml— Migrated to PyPI Trusted Publishing: addedpermissions: id-token: writeat job level, removedpassword: ${{ secrets.PYPI_API_TOKEN }}.CHANGELOG.md— New file in Keep a Changelog format with a[0.5.0]entry.pyproject.toml— AddedChangelogURL under[project.urls].Original prompt
Problems to Fix
1. Resolve merge conflict in
LICENSEThe
LICENSEfile currently contains unresolved Git merge conflict markers (<<<<<<< HEAD,=======,>>>>>>> origin/main), which means the file is corrupt. This will causetwine checkto fail and block PyPI publishing.The file contains two copies of the Apache 2.0 license text. The correct resolution is to keep a single, clean copy of the Apache 2.0 license. The correct copyright line (from the
HEADversion) is:Please replace the entire contents of
LICENSEwith a single clean copy of the Apache 2.0 license text with that copyright line. Remove all merge conflict markers.2. Upgrade publish workflow to PyPI Trusted Publishing
Update
.github/workflows/publish.ymlto use PyPI Trusted Publishing instead of thePYPI_API_TOKENsecret. This is the modern, more secure approach.The updated workflow should:
permissions: id-token: writeat the job levelpassword: ${{ secrets.PYPI_API_TOKEN }}line from thepypa/gh-action-pypi-publishsteppython -m build, check withtwine check)The final publish step should look like:
3. Add a
CHANGELOG.mdCreate a
CHANGELOG.mdin the root of the repo following Keep a Changelog format. Include an entry for the current version0.5.0with a summary of what the package does (Python SIMIND Monte Carlo connector with STIR/SIRF/PyTomography adaptors for SPECT imaging).Also add the changelog URL to
pyproject.tomlunder[project.urls]:The following is the prior conversation context from the user's chat exploration (may be truncated):
User: How close is this to being publishable on PyPI?
Assistant: This repo is very close to being publishable on PyPI — probably 85–90% of the way there. The packaging infrastructure is well set up. Here's a full breakdown:
✅ What's Already Done
pyproject.tomlsetup.pyMANIFEST.in.atnfiles)pyproject.tomlREADME.mdLICENSE[dev],[examples])pypa/gh-action-pypi-publishtwine check dist/*)__version__🚨 Blockers (Must Fix Before Publishing)
1. Git merge conflicts in
LICENSEfile (critical)The
LICENSEfile has unresolved merge conflict markers (<<<<<<< HEAD,=======,>>>>>>> origin/main). This will causetwine checkto fail or produce a broken distribution. You need to resolve this immediately.2.
PYPI_API_TOKENsecret not verifiedThe publish workflow uses
${{ secrets.PYPI_API_TOKEN }}. You'll need to either:3. Package name
py-smcmay already be taken on PyPIThe name in
pyproject.tomlispy-smc. You should verify it's available at pypi.org/project/py-smc before publishing.4. License classifier mismatch
pyproject.tomlhaslicense = "Apache-2.0"(SPDX string format, valid for modern setuptools), but GitHub shows the license as "Other" — this is likely because of the merge conflict corrupting the file. Fix theLICENSEfile and GitHub/PyPI will correctly identify it as Apache 2.0.5. No
CHANGELOG/ release notesPyPI users appreciate a changelog. Consider adding
CHANGELOG.mdand linking it inpyproject.tomlunder[project.urls].6. No
CONTRIBUTING.mdorCODE_OF_CONDUCT.mdNot required for PyPI, but good for an open-source project of this scope.
7. **Trusted Publishing (optional but recommended)...
This pull request was created from Copilot chat.
Summary by Sourcery
Resolve packaging and licensing blockers for PyPI publication.
New Features:
Enhancements: