Skip to content

Changelog profile parity - final phases#2808

Open
lcawl wants to merge 8 commits intoprofile-parityfrom
profile-parity-p3
Open

Changelog profile parity - final phases#2808
lcawl wants to merge 8 commits intoprofile-parityfrom
profile-parity-p3

Conversation

@lcawl
Copy link
Contributor

@lcawl lcawl commented Feb 27, 2026

Summary

This pull request is a continuation of #2791
It implements the final phases of an effort to get the profile-based docs-builder changelog bundle and docs-builder changelog remove invocations to parity with the command-line-option method.

I also merged some additional fixes into this PR via #2837 since at least one of them seemed like a regression.

Design Principles

  1. Mutually exclusive invocation: When using a profile (changelog bundle <profile> <arg> or changelog remove <profile> <arg>), any filter- or output-related command options must be rejected with an error. Profile config provides all such values.
  2. Allow minimal CLI overrides: Only for remove: --dry-run and --force may be used with profiles. No other CLI options are allowed when using a profile.
  3. Path derivation from config: When using a profile, paths are derived from the changelog configuration file (discovered by default, e.g. docs/changelog.yml):
    • Input directory (bundle.directory): The directory containing changelog YAML files for both bundle and remove.
    • Bundles directory (bundle.output_directory): For remove, the directory scanned for bundle dependencies. No --config or --directory override — profile invocations rely on the config file.
  4. Core functionality parity: Both invocation styles must support the same core filtering and output behaviors; profiles achieve this via config fields.

Implementation details

Phase 3 (profile-based enhancements):

  • Accept a newline-delimited URL list file as a profile argument, resolving
    to PR or issue filters based on the URLs found in the file
  • Support combined <version> <report|url-list> profile arguments so the
    version can be used for {version} substitution while the report/file
    drives filtering
  • Wire Issues filter through ProfileFilterResult into bundling and remove services

Phase 4 (option-based enhancements):

  • Add --report option to both changelog bundle and changelog remove,
    accepting a promotion report URL or local HTML file path
  • Enforce stricter validation for file-based --prs and --issues inputs:
    every line must be a fully-qualified GitHub URL

Phase 5 (tests and documentation):

  • Add 264-test coverage for all new behaviours in BundleChangelogsTests
    and ChangelogRemoveTests
  • Update changelog-bundle.md, changelog-remove.md, and contribute/changelog.md
    to document new arguments, options, validation rules, and examples

Bug fixes

Extra bugs discovered and fixed (see also #2837, which has fixes that were merged into this PR):

Bug 1: changelog bundle profile mode: bundle.directory was skipped as output fallback

ProcessProfile computed the output directory as:

bundle.output_directory → input.OutputDirectory (dead field in CLI) → input.Directory (always null in profile mode) → CWD

So when neither bundle.output_directory was set, it jumped straight to CWD, silently ignoring bundle.directory. The docs said it should use bundle.directory as the fallback, but the code didn't. Fixed to:

bundle.output_directory → input.OutputDirectory (API/test override) → bundle.directory → CWD

Bug 2: changelog remove option mode: bundle.directory was never consulted

ChangelogCommand.Remove eagerly resolved directory ?? CWD before passing it to the service. This meant ApplyConfigDefaults's bundle.directory branch was unreachable — if you had bundle.directory set in your config but didn't pass --directory, remove would always default to CWD. Fixed to pass null when --directory is absent, letting the service's fallback chain work correctly.

The consistent fallback order (now matches across both commands):

Input directory (where changelogs are read from):

Priority Profile mode Option mode
1 bundle.directory --directory
2 CWD bundle.directory
3 CWD

Output directory (bundle only — remove has no output file):

Priority Profile mode Option mode
1 bundle.output_directory --output
2 bundle.directory bundle.output_directory
3 CWD --directorybundle.directory → CWD

Bug 3: owner missing from bundle

The new owner value from bundle.owner in the changelog config was being correctly read and stored in input.Owner (via ApplyConfigDefaults), and it was being used during the matching phase (in BuildFilterCriteria.DefaultOwner) to expand bare PR numbers like 123 into owner/repo#123. However, it was never passed to BundleBuilder.BuildBundle and neither BundledProduct nor BundledProductDto had an Owner field — so it was silently dropped before YAML serialization.

Both render paths hardcode "elastic" as the GitHub owner when building PR/issue URLs from short-form numbers (e.g. 123https://github.com/elastic/{repo}/pull/123). There is currently no mechanism to override this:

Here's a summary of the fixes made:

  • Data model (bundle output)

    • BundledProductDto (Bundle.cs) — added Owner property to the YAML DTO
    • BundledProduct.cs — added Owner to domain record and constructor
    • ReleaseNotesSerialization.cs — added Owner = dto.Owner in ToBundledProduct (the ToDto side was already correct from the first session)
    • BundleBuilder.cs / ChangelogBundlingService.cs — already correct from the first session, no changes needed
  • Directive render path

    • LoadedBundle.cs — added Owner positional parameter to the record
    • BundleLoader.cs — added GetOwnerFromBundle(), passed owner to all three new LoadedBundle(...) sites
    • ChangelogInlineRenderer.cs — threaded owner through the full internal call chain (RenderSingleBundleGenerateMarkdownRenderEntriesByArea/RenderDetailedEntriesRenderSingleEntry/RenderDetailedEntryRenderEntryLinks/RenderDetailedEntryLinks) and passed it to FormatPrLink/FormatIssueLink
  • CLI render path

    • ResolvedEntry — added Owner property
    • BundleDataResolver.cs — extracts owner from bundle products, sets on each ResolvedEntry
    • ChangelogRenderContext.cs — added Owner and EntryToOwner properties
    • ChangelogRenderingService.cs — populates entryToOwner map and sets Owner/EntryToOwner on context
    • MarkdownRendererBase.csGetEntryContext now returns entryOwner; RenderPrIssueLinks now accepts and passes entryOwner
    • IndexMarkdownRenderer, KnownIssuesMarkdownRenderer, HighlightsMarkdownRenderer, DeprecationsMarkdownRenderer, BreakingChangesMarkdownRenderer — updated destructuring and call sites
  • ChangelogTextUtilities.cs — added string owner = "elastic" parameter to all four link formatters; used in markdown URL templates

  • Tests — updated BundleChangelogs_WithProfile_RepoAndOwner_WritesValuesToProductEntries to assert Contain("owner: elastic") instead of NotContain("owner:")

NOTE: The asciidoc link formatters use attribute-reference syntax ({repo-pull}123) rather than constructing GitHub URLs directly, so owner has no meaning there — the owner is baked into the asciidoc attribute definitions elsewhere. FormatPrLinkAsciidoc and FormatIssueLinkAsciidoc therefore don't need owner info. The markdown variants (FormatPrLink and FormatIssueLink) retain the owner parameter and use it in the URL template.

Steps for testing

The following sections describe the manual tests I performed.

Test profile-based bundles with PR and issue filters

  1. Check out [DOCS] Elastic Agent release note test elastic-agent#12296 as an example of small set of changelogs and a full-qualified list of PRs in a newline-delimited text file.
  2. Create a bundle based on the PR list. For example, in the elastic-agent repo, run:
    docs-builder changelog bundle elastic-agent-release 9.3.0 ./changelog/elastic-agent-9.3.0.txt
    I verified that it contained the same details as the command-option variation.

Test combined <version> <report|url-list> profile-based bundle

Refer to https://github.com/lcawl/elasticsearch-serverless/pull/1

Test new --report option to both changelog bundle and changelog remove

Refer to https://github.com/lcawl/elasticsearch-serverless/pull/1

Confirm stricter validation for file-based --prs and --issues

  1. Check out Kibana release notes test kibana#250840 as an example of changelogs and bundle commands that use a file list of PRs (short form only)
  2. Try running one of the bundle commands that use files, for example:
    docs-builder changelog bundle \
    --config ~/Documents/GitHub/kibana/docs/changelog.yml \
    --prs ~/Documents/GitHub/kibana/docs/9.3.0-kibana.txt \
    --directory ~/Documents/GitHub/kibana/docs/changelog \
    --output ~/Documents/GitHub/kibana/docs/releases/kibana/9.3.0.yaml \
    --repo kibana --owner elastic \
    --output-products "kibana 9.3.0 *" \
    --hide-features test_feature1 --resolve
    Verify that it now fails with a message like this:
    Error: File must contain fully-qualified GitHub URLs (e.g. https://github.com/owner/repo/pull/123). Numbers and short forms are not allowed. Found: 224552

Outstanding work

  • Some further cleanup of the changelog.md is necessary (for example to add profile-based examples since most of the bundle section of the doc relates to the command-options).
  • Confirm whether it's a known problem that running the "bundle" command against a buildkite URL (instead of a downloaded copy) yields Error: Failed to fetch promotion report from URL: Forbidden

Generative AI disclosure

  1. Did you use a generative AI (GenAI) tool to assist in creating this contribution?
  • Yes
  • No
  1. If you answered "Yes" to the previous question, please specify the tool(s) and model(s) used (e.g., Google Gemini, OpenAI ChatGPT-4, etc.).

Tool(s) and model(s) used: composer-1.5, claude-4.6-sonnet-medium

…ation

Phase 3 (profile-based enhancements):
- Accept a newline-delimited URL list file as a profile argument, resolving
  to PR or issue filters based on the URLs found in the file
- Support combined <version> <report|url-list> profile arguments so the
  version can be used for {version} substitution while the report/file
  drives filtering
- Wire Issues filter through ProfileFilterResult into bundling and remove services

Phase 4 (option-based enhancements):
- Add --report option to both `changelog bundle` and `changelog remove`,
  accepting a promotion report URL or local HTML file path
- Enforce stricter validation for file-based --prs and --issues inputs:
  every line must be a fully-qualified GitHub URL

Phase 5 (tests and documentation):
- Add 264-test coverage for all new behaviours in BundleChangelogsTests
  and ChangelogRemoveTests
- Update changelog-bundle.md, changelog-remove.md, and contribute/changelog.md
  to document new arguments, options, validation rules, and examples

Made-with: Cursor
@lcawl lcawl marked this pull request as ready for review March 3, 2026 05:33
@lcawl lcawl requested review from a team as code owners March 3, 2026 05:33
@lcawl lcawl requested a review from reakaleek March 3, 2026 05:33
lcawl added a commit that referenced this pull request Mar 3, 2026
…-version

Aligns with PR #2791 (bundle.repo/owner config fields) and PR #2808
(owner stored in bundle YAML and used for link generation).

Config model changes:
- BundleConfiguration gains Repo and Owner properties
- BundleConfigurationYaml and ChangelogConfigurationLoader updated to
  serialize/deserialize the new fields

Service changes:
- ChangelogBundlingService.ApplyConfigDefaults falls back to
  config.Bundle.Repo / config.Bundle.Owner when CLI flags are absent
- ChangelogRemoveService.ApplyConfigDefaults does the same

Command changes:
- Both Bundle and Remove --release-version blocks now load the changelog
  config before fetching the GitHub release, so the owner used to
  construct PR URLs follows the full precedence:
    --owner CLI > bundle.owner in changelog.yml > "elastic"
- Same fix applied to the Bundle command for consistency

Docs:
- changelog-remove.md and contribute/changelog.md document the owner
  precedence, restore the "Remove by GitHub release" section, and add
  a matching note to the bundle release-version section

Tests:
- Two new tests in ChangelogRemoveTests verify config owner fallback
  and that explicit --owner overrides the config value

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant