Remove empty sections from changelog directive#3369
Open
lcawl wants to merge 4 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR updates changelog rendering so headings, TOC anchors, and version blocks are produced only when bundles contain entries after publish-blocker, hide-features, and :type: filtering. ChangelogInlineRenderer adds helpers to classify page types, compute filtered entries, and decide when to render empty bundle metadata; it skips blank bundle outputs and returns null when nothing renders. ChangelogBlock uses filtered counts to emit anchors/TOC and respects dedicated-page semantics. Tests were updated and extended to validate omission and description-preserving rules. Sequence Diagram(s)sequenceDiagram
participant ChangelogBlock
participant ChangelogInlineRenderer
participant LoadedBundle
participant TOC
participant Output
ChangelogBlock->>ChangelogInlineRenderer: GetFilteredEntries(bundle, publishBlocker, hideFeatures, typeFilter)
ChangelogInlineRenderer->>LoadedBundle: filter entries (publishBlocker, hideFeatures, typeFilter)
LoadedBundle-->>ChangelogInlineRenderer: filtered entries list
ChangelogInlineRenderer-->>ChangelogBlock: filtered counts / renderable flag
ChangelogBlock->>TOC: ComputeTableOfContent(filtered counts, dedicatedPage)
ChangelogBlock->>ChangelogInlineRenderer: GenerateMarkdown(for each bundle)
ChangelogInlineRenderer->>Output: Render bundle markdown or skip / render metadata
ChangelogBlock->>Output: Assemble TOC + bundle sections (only non-empty)
🚥 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
🤖 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/Elastic.Markdown/Myst/Directives/Changelog/ChangelogInlineRenderer.cs`:
- Around line 735-745: The current branch uses RenderDetailedEntries (dropdown
rendering) whenever subsections is true, causing dropdowns even if
dropdownsEnabled is false; change the conditional so that when subsections is
true AND dropdownsEnabled is false you call a flattened-by-area renderer
(implement the proposed RenderDetailedEntriesFlattenedByArea or similar) instead
of RenderDetailedEntries; update the call sites to pass the same parameters
(entries, repo, owner, hideLinks, hideEntryDescriptions, publishBlocker) and
ensure RenderDetailedEntryFlattened is used to render each entry, leaving the
existing RenderDetailedEntries and RenderDetailedEntriesFlattened behavior
unchanged for other cases, and keep the original branch for dropdownsEnabled ==
true to call RenderDetailedEntries.
🪄 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: 63165254-262f-48af-8921-9e27b9d741a7
📒 Files selected for processing (6)
docs/syntax/changelog.mdsrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cssrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogInlineRenderer.cstests/Elastic.Markdown.Tests/Directives/ChangelogBasicTests.cstests/Elastic.Markdown.Tests/Directives/ChangelogTocFilteringTests.cstests/Elastic.Markdown.Tests/Directives/ChangelogTypeFilterTests.cs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Elastic.Markdown.Tests/Directives/ChangelogTypeFilterTests.cs (1)
207-225: ⚡ Quick winRename new tests to
Method_Scenario_Expectedformat.The two new test names don’t follow the test naming convention required for
tests/**/*.cs. Please rename them to theMethod_Scenario_Expectedpattern for consistency.As per coding guidelines, "Test method naming convention: Method_Scenario_Expected."
🤖 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 `@tests/Elastic.Markdown.Tests/Directives/ChangelogTypeFilterTests.cs` around lines 207 - 225, The two test methods TableOfContentsIncludesSeparatedTypeSections and SeparatedTypeTocSlugsMatchHtmlIds must be renamed to follow the Method_Scenario_Expected convention; update TableOfContentsIncludesSeparatedTypeSections to a name like GenerateTableOfContents_SeparatedTypes_IncludesSections and rename SeparatedTypeTocSlugsMatchHtmlIds to something like GenerateTableOfContents_SeparatedTypes_SlugsMatchHtmlIds (or equivalent Method_Scenario_Expected names) so the tests in Directives/ChangelogTypeFilterTests.cs conform to the project naming standard.
🤖 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.
Nitpick comments:
In `@tests/Elastic.Markdown.Tests/Directives/ChangelogTypeFilterTests.cs`:
- Around line 207-225: The two test methods
TableOfContentsIncludesSeparatedTypeSections and
SeparatedTypeTocSlugsMatchHtmlIds must be renamed to follow the
Method_Scenario_Expected convention; update
TableOfContentsIncludesSeparatedTypeSections to a name like
GenerateTableOfContents_SeparatedTypes_IncludesSections and rename
SeparatedTypeTocSlugsMatchHtmlIds to something like
GenerateTableOfContents_SeparatedTypes_SlugsMatchHtmlIds (or equivalent
Method_Scenario_Expected names) so the tests in
Directives/ChangelogTypeFilterTests.cs conform to the project naming standard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 768a26a5-05b3-4e1d-b43c-f447df7904ec
📒 Files selected for processing (2)
tests/Elastic.Markdown.Tests/Directives/ChangelogIndexPageTocTests.cstests/Elastic.Markdown.Tests/Directives/ChangelogTypeFilterTests.cs
✅ Files skipped from review due to trivial changes (1)
- tests/Elastic.Markdown.Tests/Directives/ChangelogIndexPageTocTests.cs
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.
Fixes #3368
AI Summary
Improves
{changelog}rendering for dedicated release-notes pages and removes low-value empty output on general release-notes pages. Release note pages such as known issues, deprecations, and breaking changes no longer show redundant section headings, empty dated releases, or “there are no … associated with this release” placeholders.Problem
On pages that already have an H1 and use
:type:to narrow entries (for example:type: known-issueon a known-issues page), the directive was still emitting:### Deprecations/### Breaking changes/### Known issuesunder each## {version}## {version}blocks with no matching entries, often with italic placeholder textBehavior changes
Dedicated type pages (
:type: breaking-change,deprecation,known-issue,highlight):### Deprecations, etc.)## {version}:subsections:description)General release notes (default or
:type: all):_There are no …_/_No new features…_description## {version}+ description only (e.g. pulled release)release-dateonly_Released: …_release-datedoes not preserve an empty release)hide-featuresBreaking change: Empty placeholder messages are removed with no opt-out flag.
Out of scope:
changelog renderCLI (static file output) is unchanged.Implementation notes
ChangelogInlineRenderer:IsDedicatedSeparatedTypePage,GetFilteredEntries,BundleHasRenderableEntries,ShouldRenderEmptyBundleMetadata(description only)RenderChangelogMarkdownskips whitespace-only bundle output; returnsnullwhen nothing rendersChangelogBlockTOC and anchors use the same filtering logic viaBundleContributesToNavigationRenderSeparatedTypeEntriesuses area grouping when:subsections:is set on dedicated pagesFiles changed
src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogInlineRenderer.csGetEmptyMessagesrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cstests/Elastic.Markdown.Tests/Directives/ChangelogTypeFilterTests.cstests/Elastic.Markdown.Tests/Directives/ChangelogTocFilteringTests.cstests/Elastic.Markdown.Tests/Directives/ChangelogBasicTests.csdocs/syntax/changelog.mdScreenshots
Before
Per elastic/docs-content#6599
Unnecessary deprecation headings:
Empty breaking change sections:
After
No more unnecessary deprecation headings:
No more empty breaking change sections:
Generative AI disclosure
Tool(s) and model(s) used: composer-2.5, claude-4-sonnet-thinking