feat(changelog): changelog note command for PR-less entries - #3923
feat(changelog): changelog note command for PR-less entries#3923Mpdreamz wants to merge 4 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes for one correctness issue: changelog note should apply the same post-config CI-description clearing behavior as changelog add when release-note extraction resolves to disabled.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
There was a problem hiding this comment.
Requesting changes for one correctness issue: changelog note should not implicitly inherit PR linkage from CI (CHANGELOG_PR_NUMBER) when --prs is not provided.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Adds changelog note as a sibling of changelog add. Notes write note-<slug>.yml files (keyed by name/title, not by PR) for items with no PR — such as known-issue entries that today go directly into bundle YAML. All products must have a concrete target (wildcard * is rejected). PRs/issues may still be cited but are optional and do not affect the filename. New surface: changelog note --title "..." --type known-issue --products "product 9.2.0 ga" changelog note --name tsdb-gap --products "elasticsearch 9.2.0 ga" ... Adds ValidateNoteProducts to CreateChangelogArgumentsValidator, WriteNoteAsync + GenerateNoteFilename + Slugify to ChangelogFileWriter, and CreateNote to ChangelogCreationService. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d6d443b to
c1f597a
Compare
There was a problem hiding this comment.
Requesting changes for one correctness issue in changelog note citation validation.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Numeric --prs or --issues without --owner/--repo now fail with the same error as the `add` path instead of writing ambiguous numeric references into YAML. Two regression tests added. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Requesting changes: multi-value --prs/--issues validation in changelog note still permits mixed numeric+URL inputs without repository context, which allows ambiguous bare numeric citations to be written.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| // Validate issue citation format | ||
| if (input.Issues is { Length: > 1 }) | ||
| { | ||
| if (!_validator.ValidateMultipleIssueFormat(collector, input.Issues, input.Owner, input.Repo)) |
There was a problem hiding this comment.
Same gap for issues: in the multi-value branch, ValidateMultipleIssueFormat only rejects when all entries are numeric. A mixed input like --issues 456 --issues https://github.com/elastic/elasticsearch/issues/789 can pass without repo context and persist an ambiguous bare numeric issue.
Please apply per-entry validation (or reject when any numeric value is present without owner/repo context), matching the single-value validation rule.
There was a problem hiding this comment.
Fixed in 1b42111 — same Any() fix applied to ValidateMultipleIssueFormat. New test: CreateNote_MixedNumericAndUrlIssueWithoutOwnerRepo_ReturnsError.
| // Validate PR citation format (same rule as `add`: numeric refs require --owner/--repo) | ||
| if (input.Prs is { Length: > 1 }) | ||
| { | ||
| if (!_validator.ValidateMultiplePrFormat(collector, input.Prs, input.Owner, input.Repo)) |
There was a problem hiding this comment.
CreateNote only runs ValidateMultiplePrFormat when more than one PR is passed, but that validator only fails when all entries are numeric. A mixed input like --prs 123 --prs https://github.com/elastic/elasticsearch/pull/999 can therefore pass without repo context, leaving 123 as an ambiguous citation.
Given the intended rule here (numeric refs require repo context), this should validate each PR entry (or fail when any entry is numeric and repo context is missing), not only the all-numeric case.
There was a problem hiding this comment.
Fixed in 1b42111 — changed ValidateMultiplePrFormat from All() to Any() so a single bare numeric entry in a mixed numeric+URL list is rejected when --owner/--repo are absent. New test: CreateNote_MixedNumericAndUrlPrWithoutOwnerRepo_ReturnsError.
…ion validation in CreateNote - Add pre-enrichment cliDescription capture and post-ApplyConfigDefaults extraction-disabled guard to CreateNote, mirroring the identical logic in CreateChangelog - Change ValidateMultiplePrFormat/ValidateMultipleIssueFormat from All() to Any() so a single bare numeric entry in a mixed numeric+URL list is rejected when owner/repo are absent - Add three new tests: mixed PR, mixed issue, and CI-description-clearing for CreateNote Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Approved — no blocking issues found.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
2 participants
Summary
Adds
changelog noteas a sibling ofchangelog add. Notes solve the problem of PR-less items (such as known-issue entries that today go straight into bundle YAML, bypassing the pool entirely) by giving them a pool-native, scrubbable home.note-<slug>.yml— slug from--nameor slugified title, never a PR number*is rejected). This is enforced at creation time and is the key difference fromadd.changelog uploadpicks upnote-*.ymlfiles verbatim (no change needed — the key derives from the filename, which is already correct)Stacked on #3922 (groundwork for PR-anchored CDN sourcing). That PR must merge first.
New surface
Test plan
dotnet test tests/Elastic.Changelog.Tests/)note-<slug>.ymlcreated with title-derived slug when--nameabsent--name tsdb-gap→note-tsdb-gap.ymltarget: "*"→ error pointing at concrete target🤖 Generated with Claude Code