Repository navigation
feat(changelog-preview): Keep previews for different directories apart - #924
sentry-junior[bot] wants to merge 5 commits into
Conversation
…ws apart Co-Authored-By: Lorenzo Cian <17258265+lcian@users.noreply.github.com>
|
Co-Authored-By: Lorenzo Cian <17258265+lcian@users.noreply.github.com>
BYK
left a comment
There was a problem hiding this comment.
Ah, sounds great and thanks for the PR! I wonder if we can share a single PR with multiple change logs, collapsed by default? Looks possible with your markers which we can even move into the <details> elements or something or workspace headers?
MathurAditya724
left a comment
There was a problem hiding this comment.
This adds useful monorepo preview isolation that is separate from the AI-summary work merged in #929. Two follow-ups below: avoid directory-identity collisions and preserve the behavior in automated regression tests.
…t the preview step
MathurAditya724
left a comment
There was a problem hiding this comment.
Both earlier comments are addressed: normalization plus a hash separates sanitized paths, and committed fixture tests execute the actual preview step. The seven committed tests pass locally, with typechecking, ESLint and formatting also passing. Additional isolated probes passed for equivalent directory spellings, unsafe/Unicode names and CalVer behavior. One remaining status-mode collision is documented inline: GitHub compares status contexts case-insensitively, unlike comment markers. No other actionable correctness or DRY issues found in the three-file diff.
…survive case-insensitive status contexts
Gives each changelog preview its own PR comment and status check, keyed by the workflow's
working-directory, so one PR can carry a preview per package without them overwriting each other. Callers don't need to configure anything new.Use case: one PR that touches two packages
Some repos release several packages from separate Craft configs. For example, getsentry/objectstore has a Python client and a Rust client, each with its own
.craft.yml. The natural setup calls the preview workflow once per package:Now take a single PR that changes both clients, e.g. one that adds a new option to both.
Expected: two previews on the PR, one with the Python changelog entry and bump, one with the Rust one.
What happens today: both calls use the same hardcoded identity.
craft-changelog-previewHTML marker. The second run edits the first run's comment, so only one preview survives, and nothing says which package it's for.comment: false): both post the commit status contextChangelog Preview / Semver Impact. The last one wins, so the PR shows a singleMinororPatchwithout saying which package it applies to. The other package's preview is only visible if you open its Actions run.The change
The preview ID is the working directory's repo-relative path, as reported by
git rev-parse --show-prefix. Git normalizes it, soclients/python,./clients/python/and so on all give the same ID.., the default)working-directory: clients/pythoncraft-changelog-preview(unchanged)craft-changelog-preview:clients/pythonChangelog Preview / Semver Impact(unchanged)Changelog Preview / Semver Impact (clients/python)Semver Impact of This PR(unchanged)Semver Impact of This PR (clients/python)Two packages always run from different directories, since each has its own
.craft.yml, so the directory is a natural key and there's no new input to learn.[A-Za-z0-9._/-]is replaced with-. When that changes the path, the first 8 characters of a hash of the real path are appended, e.g.packages/foo barbecomespackages/foo-bar~1a2b3c4d. So it stays separate from a realpackages/foo-bar.packages/Pythonandpackages/pythonwould still share a status. Paths with uppercase letters get the hash too (packages/Python~…), so the IDs differ even after case folding.~is never kept, so a hashed ID can't match an unhashed one.COMMENT_MARKER) and used for both writing and looking up the comment.docs/src/content/docs/github-actions.md, and theworking-directoryinput description mentions it.What existing callers will notice
These only apply to callers that already set a non-root
working-directory:Changelog Preview / Semver Impacta required status, it needs updating. The status is alwayssuccess, so I'd expect that to be rare.Testing
src/__tests__/changelog-preview-workflow.test.tsruns the real "Generate Changelog Preview" step from the workflow file, in a temp git repo. It uses a stubbedcraftand a fakeghthat stores comments and statuses in JSON files. Lookups go through the step's own--jqfilter. Like GitHub, the fake replaces any status whose context matches case-insensitively. It checks that:packages/pythonandpackages/rusteach create and then update only their own comment, next to the root's;packages/pythonandpackages/Pythonkeep two separate statuses. This test is skipped on case-insensitive file systems, since both directories can't exist there;packages/foo barandpackages/foo-barget different, stable markers.The case test fails without this change and passes with it; the other directory tests fail against
master's workflow.actionlint,prettier,eslintandtscare clean. I haven't run it on a real PR.Details
via Junior system actor
event.--
View Junior Session [Sentry]