[Release notes] Add bundle-amend --remove option#3513
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 19 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds the ability to remove changelog entries from published bundles via a new Sequence DiagramsequenceDiagram
rect rgba(100, 149, 237, 0.5)
note over CLI,FS: bundle-amend --remove flow
end
participant CLI as docs-builder CLI
participant ChangelogCommand
participant AmendService as ChangelogBundleAmendService
participant Merger as BundleAmendMerger
participant FS as FileSystem / YAML
CLI->>ChangelogCommand: bundle-amend --remove file.yaml [--force] [--dry-run]
ChangelogCommand->>ChangelogCommand: normalize paths, validate at least one of --add/--remove
ChangelogCommand->>AmendService: AmendBundle(AmendBundleArguments)
AmendService->>FS: deserialize parent bundle + existing amend bundles
AmendService->>Merger: CollectAppliedExclusionKeys(existingAmends)
loop each RemoveFile
AmendService->>FS: read file, compute checksum
AmendService->>Merger: EntryMatchesExclusion(entry, exclusion)
AmendService->>Merger: BuildExclusionKey(exclusion)
AmendService-->>AmendService: BuildExclusionEntryAsync → Add | Skip | fail
end
AmendService->>Merger: MergeEntries(parentEntries, existingAmends)
alt --dry-run
AmendService-->>CLI: return success (no write)
else write amend
AmendService->>FS: write .amend-N.yaml with exclude-entries + entries
AmendService-->>CLI: return success
end
🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/syntax/changelog.md (1)
322-326:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't recommend
--removeas a fix for an already-missing file.The PR notes this path still fails once the changelog file is no longer on disk. For that case, tell readers to restore the file or re-bundle with
--resolve;bundle-amend --removeonly applies while the source changelog is still available.🤖 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 `@docs/syntax/changelog.md` around lines 322 - 326, The documentation in the changelog.md file currently recommends using `bundle-amend --remove` as a general fix for missing changelog files, but this option only works when the source changelog file is still available on disk. Remove the `--remove` option from the list of fixes in the bullet points, or alternatively, add a note clarifying that this option only applies when the source file exists. Keep only the two viable fixes for missing files: restoring the missing changelog files or re-creating the bundle with `--resolve` to embed entry content directly.
🤖 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.Documentation.Configuration/ReleaseNotes/BundleLoader.cs`:
- Around line 281-313: Orphan amend files that don't have a corresponding parent
bundle are being included in the final bundle output as standalone bundles
because they are not added to mergedAmendPaths when the parent lookup fails.
When the bundlesByPath.TryGetValue check fails for parentPath and the continue
statement executes at line 288, you must still add all amend files in the
current group to mergedAmendPaths before continuing, so that these orphan amend
files are properly excluded from the final returned bundle list.
In `@src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs`:
- Around line 341-349: In the LoadExistingAmendBundlesAsync method, the call to
ReleaseNotesSerialization.DeserializeBundle on line 348 lacks error handling for
malformed YAML. Wrap the ReadAllTextAsync and DeserializeBundle calls in a
try-catch block to catch deserialization exceptions, log the error for
diagnostics purposes, and continue processing the remaining amend files instead
of allowing the exception to propagate and crash the service.
In `@src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs`:
- Around line 360-366: The deserialization of individual amend files using
ReleaseNotesSerialization.DeserializeBundle at lines 360-366 is not protected,
so if one amend file fails to deserialize, the exception propagates to the outer
catch block and skips dependency analysis for the entire parent bundle,
potentially allowing deletion of required files. Wrap the
ReleaseNotesSerialization.DeserializeBundle call in a try-catch block within the
loop over amendPaths so that individual deserialization failures are handled
gracefully without preventing the parent bundle's dependency analysis. Only add
valid deserialized bundles to amendBundles and continue processing remaining
amend files. Apply the same fix to the second location mentioned at lines
394-397.
---
Outside diff comments:
In `@docs/syntax/changelog.md`:
- Around line 322-326: The documentation in the changelog.md file currently
recommends using `bundle-amend --remove` as a general fix for missing changelog
files, but this option only works when the source changelog file is still
available on disk. Remove the `--remove` option from the list of fixes in the
bullet points, or alternatively, add a note clarifying that this option only
applies when the source file exists. Keep only the two viable fixes for missing
files: restoring the missing changelog files or re-creating the bundle with
`--resolve` to embed entry content directly.
🪄 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: 794f960c-9f1a-4c58-8bf0-efb55995e9c3
📒 Files selected for processing (19)
docs/cli-schema.jsondocs/cli/changelog/cmd-bundle-amend.mddocs/contribute/bundle-changelogs.mddocs/syntax/changelog.mdsrc/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cssrc/Elastic.Documentation.Configuration/ReleaseNotes/BundleAmendMerger.cssrc/Elastic.Documentation.Configuration/ReleaseNotes/BundleLoader.cssrc/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cssrc/Elastic.Documentation/ReleaseNotes/Bundle.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cssrc/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cssrc/services/Elastic.Changelog/Rendering/BundleValidationService.cssrc/tooling/docs-builder/Commands/ChangelogCommand.cstests/Elastic.Changelog.Tests/Changelogs/BundleAmendMergerTests.cstests/Elastic.Changelog.Tests/Changelogs/BundleAmendTests.cstests/Elastic.Changelog.Tests/Changelogs/BundleLoading/BundleLoaderTests.cstests/Elastic.Changelog.Tests/Changelogs/ChangelogRemoveTests.cstests/Elastic.Changelog.Tests/Changelogs/Render/BundleValidationTests.cstests/Elastic.Markdown.Tests/Directives/ChangelogBasicTests.cs
1 participant
Summary
Add a
--removeoption to thedocs-builder changelog bundle-amendcommand, so that changelogs can be removed from bundles post-release if necessary.Previously, the bundle had to be manually edited or entirely replaced.
Core changes
Schema — Added
exclude-entriestoBundle.cs,BundleDto, andReleaseNotesSerialization.cs.BundleAmendMerger— New shared merger inBundleAmendMerger.cs: per amend file, apply exclusions then additions; ordered by amend number.Merge wiring — Updated
BundleLoader.csandBundleValidationService.csto use the merger.ChangelogBundleAmendService— Supports--remove,--force, and--dry-run; writesexclude-entriesto new.amend-N.yamlsidecars; checksum matching with optional--forcefor name-only removal.CLI —
ChangelogCommand.BundleAmendaccepts--remove,--force,--dry-run; requires at least one of--addor--remove.changelog remove—ChangelogRemoveServiceuses the effective bundle (parent + amends − exclusions) for dependency checks; skips amend files as parent bundle candidates.Tests added
BundleAmendMergerTests.csBundleAmendTests.csBundleLoaderTests.csChangelogRemoveTests.cschangelog renderomits excluded entry inBundleValidationTests.cs{changelog}directive omits excluded entry and merged bundle has one entry inChangelogBasicTests.csExample usage
Steps for testing
I tested this command as follows:
changelog bundle-amendcommand to remove a changelog. For example, I tested against a bundle with a single changelog in the aforementioned repo:Error: Bundle contains 'upstream-update.yaml' but with a different checksum than the file on disk. Re-create the bundle or use --force to remove by file name only. NOTE: /path/elastic-otel-java/docs/changelog/upstream-update.yaml--forceoption, it successfully created adocs/releases/1.11.0.amend-1.yamlfile with the following contents:NOTE: If you try to remove a changelog from a bundle but you don't have access to the changelog file anymore, you get an error like this:
Generative AI disclosure
Tool(s) and model(s) used: composer-2.5-fast