Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds a new --dropdowns CLI option and a Dropdowns flag propagated through RenderChangelogsArguments -> ChangelogRenderingService -> ChangelogRenderContext. Markdown renderers (BreakingChanges, Deprecations, Highlights, KnownIssues) now branch on context.Dropdowns to emit either MyST dropdown blocks or flattened bullet/list output. MarkdownRendererBase introduces PrIssueLinkOptions to support indentation for list-style PR/issue rendering. AsciiDoc renderers were adjusted for list-continuation, invariant-culture headers, and conditional subsection grouping; docs and tests were added/updated. Sequence DiagramsequenceDiagram
participant User
participant CLI as ChangelogCommand
participant Service as ChangelogRenderingService
participant Context as ChangelogRenderContext
participant Renderers as MarkdownRenderers
participant Output
User->>CLI: run changelog render --dropdowns
CLI->>Service: RenderChangelogsArguments(dropdowns)
Service->>Service: SetupOutput() (title/slug)
Service->>Context: initialize (Dropdowns = input.Dropdowns)
Service->>Renderers: render each section with Context
alt Dropdowns = true
Renderers->>Output: emit MyST dropdown blocks
else Dropdowns = false
Renderers->>Output: emit flattened bullet lists (indented links)
end
Output->>User: rendered files (md or asciidoc)
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/Elastic.Changelog/Rendering/Markdown/MarkdownRendererBase.cs (1)
42-48: ⚡ Quick winConsider folding the parameters into a small record.
Both overloads now exceed the 4-parameter cap, and call sites pass
entryHideLinkspositionally next to a second boolindentForListItem. A smallPrIssueLinkOptions(ChangelogEntry Entry, string Repo, string Owner, bool HideLinks, bool IndentForListItem)would collapse this toRenderPrIssueLinks(sb, options)and remove the bool-adjacency at call sites.As per coding guidelines: "Method parameters should not exceed 4 in count. Use a record or options object for methods that require more than 4 parameters. Boolean parameters must be named at call sites."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/Elastic.Changelog/Rendering/Markdown/MarkdownRendererBase.cs` around lines 42 - 48, The RenderPrIssueLinks overloads exceed the 4-parameter guideline and pass two adjacent bools positionally; create a small record (e.g., PrIssueLinkOptions with properties ChangelogEntry Entry, string Repo, string Owner, bool HideLinks, bool IndentForListItem) and change both method signatures to RenderPrIssueLinks(StringBuilder sb, PrIssueLinkOptions options); update internal references inside MarkdownRendererBase to use options.* and update all call sites to pass a named PrIssueLinkOptions instance (ensuring bools are named via properties) so callers become RenderPrIssueLinks(sb, new PrIssueLinkOptions { Entry = entry, Repo = entryRepo, Owner = entryOwner, HideLinks = entryHideLinks, IndentForListItem = indentForListItem }); ensure overload forwarding is removed or adjusted accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Elastic.Changelog.Tests/Changelogs/Render/DropdownRenderTests.cs`:
- Around line 419-422: The test asserts AsciiDoc uses unconstrained strong
markers (“**Impact:**”, “**Action:**”) which is non-idiomatic; update the
DropdownRenderTests.cs assertions to expect idiomatic constrained strong markers
("*Impact:*", "*Action:*") or make the assertions tolerant (e.g., regex or
alternatives) to accept either single- or double-asterisk forms so the test
stays valid if AsciidocRendererBase.cs (where the renderer currently emits
"**Impact:**" / "**Action:**" around lines 91 and 97) is changed to emit the
idiomatic single-asterisk format.
---
Nitpick comments:
In `@src/services/Elastic.Changelog/Rendering/Markdown/MarkdownRendererBase.cs`:
- Around line 42-48: The RenderPrIssueLinks overloads exceed the 4-parameter
guideline and pass two adjacent bools positionally; create a small record (e.g.,
PrIssueLinkOptions with properties ChangelogEntry Entry, string Repo, string
Owner, bool HideLinks, bool IndentForListItem) and change both method signatures
to RenderPrIssueLinks(StringBuilder sb, PrIssueLinkOptions options); update
internal references inside MarkdownRendererBase to use options.* and update all
call sites to pass a named PrIssueLinkOptions instance (ensuring bools are named
via properties) so callers become RenderPrIssueLinks(sb, new PrIssueLinkOptions
{ Entry = entry, Repo = entryRepo, Owner = entryOwner, HideLinks =
entryHideLinks, IndentForListItem = indentForListItem }); ensure overload
forwarding is removed or adjusted accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0d9d1f38-3e96-48e5-a1c5-c0755a97fcaa
📒 Files selected for processing (11)
docs/cli/changelog/render.mdsrc/services/Elastic.Changelog/Rendering/ChangelogRenderContext.cssrc/services/Elastic.Changelog/Rendering/ChangelogRenderingService.cssrc/services/Elastic.Changelog/Rendering/Markdown/BreakingChangesMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/DeprecationsMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/HighlightsMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/KnownIssuesMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/MarkdownRendererBase.cssrc/tooling/docs-builder/Commands/ChangelogCommand.cstests/Elastic.Changelog.Tests/Changelogs/Render/DropdownRenderTests.cstests/Elastic.Changelog.Tests/Changelogs/Render/TitleTargetTests.cs
2 tasks
* Improve changelog render asciidoc output * Remove definition lists in favour of subsections
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/Elastic.Changelog/Rendering/Asciidoc/DeprecationsAsciidocRenderer.cs (1)
31-34: 💤 Low valueUnreachable
"General"fallback — dead code.The ternary on line 33 is inside
if (context.Subsections && !string.IsNullOrWhiteSpace(group.Key)), sogroup.Keyis already guaranteed to be non-empty; the"General"branch can never execute. Same dead code appears inEntriesByAreaAsciidocRenderer.cs,HighlightsAsciidocRenderer.cs, andKnownIssuesAsciidocRenderer.cs.🔧 Proposed fix
- var componentName = group.Key != string.Empty ? group.Key : "General"; + var componentName = group.Key;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/Elastic.Changelog/Rendering/Asciidoc/DeprecationsAsciidocRenderer.cs` around lines 31 - 34, The ternary that assigns componentName is dead because the surrounding if already ensures !string.IsNullOrWhiteSpace(group.Key); update the code in DeprecationsAsciidocRenderer (and the same pattern in EntriesByAreaAsciidocRenderer, HighlightsAsciidocRenderer, KnownIssuesAsciidocRenderer) to remove the unreachable branch and simply use group.Key (or assign var componentName = group.Key) when calling ChangelogTextUtilities.FormatAreaHeader; keep the existing null/whitespace guard (context.Subsections && !string.IsNullOrWhiteSpace(group.Key)) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/services/Elastic.Changelog/Rendering/Asciidoc/DeprecationsAsciidocRenderer.cs`:
- Around line 20-22: The code uses entries.GroupBy(...).First() when
context.Subsections is false which throws if entries is empty; update the logic
in DeprecationsAsciidocRenderer (and the same pattern in
EntriesByAreaAsciidocRenderer, HighlightsAsciidocRenderer,
KnownIssuesAsciidocRenderer) so that when context.Subsections is false you
return a single grouping with Key = string.Empty and an empty sequence instead
of calling First(); specifically replace the ternary branch that uses
entries.GroupBy(_ => string.Empty).First() with a safe alternative that
constructs/returns one empty IGrouping<string, T> (or a one-element list
containing an empty group) when entries has no elements, keeping the
groupedEntries variable type unchanged. Ensure you reference the groupedEntries
variable and the context.Subsections branch when making the change.
---
Nitpick comments:
In
`@src/services/Elastic.Changelog/Rendering/Asciidoc/DeprecationsAsciidocRenderer.cs`:
- Around line 31-34: The ternary that assigns componentName is dead because the
surrounding if already ensures !string.IsNullOrWhiteSpace(group.Key); update the
code in DeprecationsAsciidocRenderer (and the same pattern in
EntriesByAreaAsciidocRenderer, HighlightsAsciidocRenderer,
KnownIssuesAsciidocRenderer) to remove the unreachable branch and simply use
group.Key (or assign var componentName = group.Key) when calling
ChangelogTextUtilities.FormatAreaHeader; keep the existing null/whitespace guard
(context.Subsections && !string.IsNullOrWhiteSpace(group.Key)) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b8a791ce-5800-4d0f-93df-69da4440ad32
📒 Files selected for processing (13)
docs/cli/changelog/render.mdsrc/services/Elastic.Changelog/Rendering/Asciidoc/AsciidocRendererBase.cssrc/services/Elastic.Changelog/Rendering/Asciidoc/BreakingChangesAsciidocRenderer.cssrc/services/Elastic.Changelog/Rendering/Asciidoc/DeprecationsAsciidocRenderer.cssrc/services/Elastic.Changelog/Rendering/Asciidoc/EntriesByAreaAsciidocRenderer.cssrc/services/Elastic.Changelog/Rendering/Asciidoc/HighlightsAsciidocRenderer.cssrc/services/Elastic.Changelog/Rendering/Asciidoc/KnownIssuesAsciidocRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/BreakingChangesMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/DeprecationsMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/HighlightsMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/KnownIssuesMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/MarkdownRendererBase.cstests/Elastic.Changelog.Tests/Changelogs/Render/DropdownRenderTests.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/services/Elastic.Changelog/Rendering/Markdown/HighlightsMarkdownRenderer.cs
- src/services/Elastic.Changelog/Rendering/Markdown/BreakingChangesMarkdownRenderer.cs
- docs/cli/changelog/render.md
- tests/Elastic.Changelog.Tests/Changelogs/Render/DropdownRenderTests.cs
- src/services/Elastic.Changelog/Rendering/Markdown/DeprecationsMarkdownRenderer.cs
…AsciidocRenderer.cs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR relates to #3228
In particular, it adds the ability to turn off dropdowns in the "changelog render" output, same as in the changelog directive.
Summary
Users can now use
--dropdownsto render separated types (breaking changes, deprecations, known issues, highlights) as MyST dropdowns, or omit the flag for the new default flattened bulleted list format.Reviewer notes
AI implementation details
All functionality has been successfully implemented according to the plan:
🔹 CLI Command: Added
--dropdownsparameter withfalsedefault🔹 Service Layer: Extended with
Dropdownsproperty throughout the rendering pipeline🔹 All Renderers: Updated to support both dropdown and flattened modes
🔹 Documentation: Added comprehensive CLI documentation
🔹 Tests: Created full test coverage for both rendering modes
There was also an issue with the way content was rendered in the non-dropdown mode when the bulleted item spanned multiple paragraphs. The output is now fixed to work as described in https://elastic.github.io/docs-builder/syntax/lists/#content-within-a-list-item:
Before (broken):
After (fixed):
Tests
I tested with some made-up changelogs in the kibana repo as follows:
--dropdownsoption, for example:--dropdownscommand option. Preview and verify that it's properly formatted:Generative AI disclosure
Tool(s) and model(s) used: composer-2, claude-4-sonnet-thinking