Accept full pull-request URLs in entry metadata - #40
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67a0d092ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match = GITHUB_PR_URL_RE.match(text) | ||
| if match: | ||
| # Keep the URL verbatim: it may point at a different repository | ||
| # than the configured one, which is the whole point of allowing it. | ||
| return PrRef(number=int(match.group("number")), url=text) |
There was a problem hiding this comment.
When a foreign-repository PR is supplied through the supported creation surfaces (tenzir-ship add --pr <URL> or Changelog.add(prs=[<URL>])), src/tenzir_ship/cli/_add.py:276-283 still passes every value to int() and raises “must be numeric,” so this parser only benefits hand-edited entry files. The bundled entry workflow likewise documents only the add --pr path in skills/tenzir-ship/references/add-changelog-entry.md:160-177; route entry creation through the URL-aware parsing/storage path and update that reference so users can actually create the newly supported metadata.
AGENTS.md reference: AGENTS.md:L52-L57
Useful? React with 👍 / 👎.
A bare `prs: [123]` is only meaningful relative to `config.repository`, so an entry cannot reference a pull request in any other repository. That became a problem for the Tenzir monorepo: its changelog holds entries whose pull requests live in tenzir/tenzir alongside newer ones from tenzir/mono, and one configured repository cannot describe both. Accept `https://github.com/<owner>/<repo>/pull/<number>` wherever a number is accepted today. Explicit URLs are kept verbatim, so they keep pointing at the repository they name, while bare numbers still resolve against `config.repository` exactly as before. Parsing now returns `PrRef(number, url)` pairs through `_parse_pr_refs`. `_build_prs_structured` and the `--explicit-links` renderer consume those so a URL survives rendering instead of being rebuilt from config. `_parse_pr_numbers` keeps its `list[int]` contract for the display-only call sites, which continue to show `#<number>` regardless of form. Previously a URL was silently dropped: the list branch skipped anything failing `str.isdigit()`, so migrating a changelog to URLs would have quietly erased every pull-request reference from rendered notes rather than failing loudly. Assisted-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d095a3633d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match = GITHUB_PR_URL_RE.match(text) | ||
| if match: | ||
| # Keep the URL verbatim: it may point at a different repository | ||
| # than the configured one, which is the whole point of allowing it. | ||
| return PrRef(number=int(match.group("number")), url=text) |
There was a problem hiding this comment.
When an entry uses the newly accepted full GitHub PR URL, validate_entry() still validates it against schemas/changelog-entry.schema.json, whose prs string pattern accepts only numeric references such as #42. Consequently, tenzir-ship validate reports every URL-backed entry as invalid, so changelogs cannot adopt this metadata while retaining validation or release quality gates; extend the entry schema and its validation tests to accept the same URL syntax as this parser.
Useful? React with 👍 / 👎.
1 participant
🔍 Problem
prs: [123]is only meaningful relative toconfig.repository, so an entry cannot reference a pull request in any other repository.The Tenzir monorepo needs exactly that. Its changelog holds entries whose pull requests live in
tenzir/tenzirnext to newer ones fromtenzir/mono, and a single configured repository cannot describe both. The current workaround is to drop PR provenance from new entries entirely.Worth flagging the existing failure mode: a URL is silently dropped today. The list branch skips anything failing
str.isdigit(), so migrating a changelog to URLs would quietly erase every PR reference from rendered notes rather than failing loudly.🛠️ Solution
Accept
https://github.com/<owner>/<repo>/pull/<number>wherever a number is accepted:config.repository, unchanged.PrRef(number, url)pairs via_parse_pr_refs;_build_prs_structuredand the--explicit-linksrenderer consume those instead of rebuilding a URL from config._parse_pr_numberskeeps itslist[int]contract for the four display-only call sites, which show#<number>regardless of input form.💬 Review
pr: 5,prs: 5,prs: ["#5"],prs: [5, 6], comma-free strings — flows through the same normalization. All 203 existing tests pass untouched.boolis rejected explicitly before theintbranch, sinceisinstance(True, int)would otherwise yield PR#1.#6441for a foreign-repo URL rather thantenzir/tenzir#6441. Changing the label is a separate call; if you'd prefer the qualified form for cross-repo references, that's a one-line change inPrRef.label.show -m --explicit-linksasserting atenzir/tenzirURL is not rewritten to the configuredtenzir/mono.ruff checkandruff formatclean.Consumer note: this needs a PyPI release before
tenzir/monomigrates its changelog. mono's CI runsuvx tenzir-ship --root engine validateand the release workflow doesuv tool install tenzir-ship, both resolving the published version — so an unreleased fix onmainwould not protect the migration.