Skip to content

Commit 4ec5cd8

Browse files
cotticotti
committed
Apply suggested fixes
1 parent f6664c5 commit 4ec5cd8

11 files changed

Lines changed: 199 additions & 88 deletions

File tree

‎config/changelog.example.yml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,9 @@ bundle:
215215
# Optional: default description text for bundles. Supports {version}, {lifecycle}, {owner}, and {repo} placeholders.
216216
# Use YAML literal block scalar (|) for multiline descriptions. See docs/contribute/changelog.md for examples.
217217
# description: |
218-
# This release includes new features and bug fixes.
219-
#
220-
# For more information, see the [release notes](https://www.elastic.co/docs/release-notes/product#product-{version}).
218+
# This release includes new features and bug fixes.
219+
#
220+
# For more information, see the [release notes](https://www.elastic.co/docs/release-notes/product#product-{version}).
221221
# changelog-init-bundle-seed
222222
# PR/issue link allowlist: when set (including []), only links to these owner/repo pairs are kept
223223
# in bundle output; others are rewritten to '# PRIVATE:' sentinels (requires resolve: true).
@@ -253,11 +253,11 @@ bundle:
253253
# # output_products: "elasticsearch {version}"
254254
# # Optional: profile-specific description (overrides bundle.description)
255255
# # description: |
256-
# # Elasticsearch {version} includes:
257-
# # - Performance improvements
258-
# # - Bug fixes and stability enhancements
259-
# #
260-
# # Download the release binaries: https://github.com/{owner}/{repo}/releases/tag/v{version}
256+
# # Elasticsearch {version} includes:
257+
# # - Performance improvements
258+
# # - Bug fixes and stability enhancements
259+
# #
260+
# # Download the release binaries: https://github.com/{owner}/{repo}/releases/tag/v{version}
261261
# Example: GitHub release profile (fetches PR list directly from a GitHub release)
262262
# Use when you want to bundle or remove changelogs based on a published GitHub release.
263263
# elasticsearch-gh-release:

‎src/Elastic.Documentation.Configuration/ReleaseNotes/BundleLoader.cs‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -217,20 +217,11 @@ private static LoadedBundle MergeBundleGroup(IGrouping<string, LoadedBundle> gro
217217
// Use the first bundle's metadata as the base
218218
var first = bundlesList[0];
219219

220-
// Merge descriptions from all bundles
221-
var descriptions = bundlesList
220+
// Use first description only; multi-bundle description merging is not yet supported
221+
var mergedDescription = bundlesList
222222
.Select(b => b.Data?.Description)
223-
.Where(d => !string.IsNullOrEmpty(d))
224-
.ToList();
225-
226-
var mergedDescription = descriptions.Count switch
227-
{
228-
0 => null,
229-
1 => descriptions[0],
230-
_ => string.Join("\n\n", descriptions)
231-
};
223+
.FirstOrDefault(d => !string.IsNullOrEmpty(d));
232224

233-
// Create merged bundle data with combined description
234225
var mergedData = first.Data with { Description = mergedDescription };
235226

236227
return new LoadedBundle(

‎src/services/Elastic.Changelog/Bundling/BundleDescriptionSubstitution.cs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Licensed to Elasticsearch B.V under one or more agreements.
2+
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
3+
// See the LICENSE file in the project root for more information
4+
5+
namespace Elastic.Changelog.Bundling;
6+
7+
/// <summary>
8+
/// Helper for performing placeholder substitution in bundle descriptions.
9+
/// Supports {version}, {lifecycle}, {owner}, and {repo} placeholders.
10+
/// </summary>
11+
internal static class BundleDescriptionSubstitution
12+
{
13+
/// <summary>
14+
/// Substitutes placeholders in a description string with provided values.
15+
/// </summary>
16+
/// <param name="description">The description string containing placeholders</param>
17+
/// <param name="version">Version value for {version} placeholder</param>
18+
/// <param name="lifecycle">Lifecycle value for {lifecycle} placeholder</param>
19+
/// <param name="owner">Owner value for {owner} placeholder</param>
20+
/// <param name="repo">Repository value for {repo} placeholder</param>
21+
/// <param name="validateResolvable">If true, validates that all used placeholders can be resolved</param>
22+
/// <returns>Description with placeholders substituted</returns>
23+
/// <exception cref="InvalidOperationException">When validateResolvable is true and placeholders cannot be resolved</exception>
24+
public static string SubstitutePlaceholders(
25+
string description,
26+
string? version,
27+
string? lifecycle,
28+
string? owner,
29+
string? repo,
30+
bool validateResolvable = false)
31+
{
32+
if (string.IsNullOrEmpty(description))
33+
return description;
34+
35+
if (validateResolvable)
36+
{
37+
var missingValues = new List<string>();
38+
if (description.Contains("{version}") && string.IsNullOrEmpty(version))
39+
missingValues.Add("version");
40+
if (description.Contains("{lifecycle}") && string.IsNullOrEmpty(lifecycle))
41+
missingValues.Add("lifecycle");
42+
if (description.Contains("{owner}") && string.IsNullOrEmpty(owner))
43+
missingValues.Add("owner");
44+
if (description.Contains("{repo}") && string.IsNullOrEmpty(repo))
45+
missingValues.Add("repo");
46+
47+
if (missingValues.Count > 0)
48+
throw new InvalidOperationException($"Cannot resolve placeholders: {string.Join(", ", missingValues)}");
49+
}
50+
51+
return description
52+
.Replace("{version}", version ?? string.Empty)
53+
.Replace("{lifecycle}", lifecycle ?? string.Empty)
54+
.Replace("{owner}", owner ?? string.Empty)
55+
.Replace("{repo}", repo ?? string.Empty);
56+
}
57+
}

‎src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs‎

Lines changed: 11 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -23,57 +23,6 @@
2323

2424
namespace Elastic.Changelog.Bundling;
2525

26-
/// <summary>
27-
/// Helper for performing placeholder substitution in bundle descriptions
28-
/// </summary>
29-
internal static class BundleDescriptionSubstitution
30-
{
31-
/// <summary>
32-
/// Substitutes placeholders in a description string with provided values
33-
/// </summary>
34-
public static string SubstitutePlaceholders(string description, string? version, string? lifecycle, string? owner, string? repo) =>
35-
SubstitutePlaceholders(description, version, lifecycle, owner, repo, validateResolvable: false);
36-
37-
/// <summary>
38-
/// Substitutes placeholders in a description string with provided values
39-
/// </summary>
40-
/// <param name="description">The description string containing placeholders</param>
41-
/// <param name="version">Version value for {version} placeholder</param>
42-
/// <param name="lifecycle">Lifecycle value for {lifecycle} placeholder</param>
43-
/// <param name="owner">Owner value for {owner} placeholder</param>
44-
/// <param name="repo">Repository value for {repo} placeholder</param>
45-
/// <param name="validateResolvable">If true, validates that all used placeholders can be resolved</param>
46-
/// <returns>Description with placeholders substituted</returns>
47-
/// <exception cref="InvalidOperationException">When validateResolvable is true and placeholders cannot be resolved</exception>
48-
public static string SubstitutePlaceholders(string description, string? version, string? lifecycle, string? owner, string? repo, bool validateResolvable)
49-
{
50-
if (string.IsNullOrEmpty(description))
51-
return description;
52-
53-
if (validateResolvable)
54-
{
55-
var missingValues = new List<string>();
56-
if (description.Contains("{version}") && string.IsNullOrEmpty(version))
57-
missingValues.Add("version");
58-
if (description.Contains("{lifecycle}") && string.IsNullOrEmpty(lifecycle))
59-
missingValues.Add("lifecycle");
60-
if (description.Contains("{owner}") && string.IsNullOrEmpty(owner))
61-
missingValues.Add("owner");
62-
if (description.Contains("{repo}") && string.IsNullOrEmpty(repo))
63-
missingValues.Add("repo");
64-
65-
if (missingValues.Count > 0)
66-
throw new InvalidOperationException($"Cannot resolve placeholders: {string.Join(", ", missingValues)}");
67-
}
68-
69-
return description
70-
.Replace("{version}", version ?? string.Empty)
71-
.Replace("{lifecycle}", lifecycle ?? string.Empty)
72-
.Replace("{owner}", owner ?? string.Empty)
73-
.Replace("{repo}", repo ?? string.Empty);
74-
}
75-
}
76-
7726
/// <summary>
7827
/// Arguments for the BundleChangelogs method
7928
/// </summary>
@@ -385,18 +334,24 @@ public async Task<bool> BundleChangelogs(IDiagnosticsCollector collector, Bundle
385334
// Apply description with placeholder substitution
386335
if (!string.IsNullOrEmpty(input.Description))
387336
{
388-
// Get version/lifecycle from the first output product or first product in the bundle
389337
var version = (input.OutputProducts?.Count > 0 ? input.OutputProducts[0].Target : null)
390338
?? (bundleData.Products.Count > 0 ? bundleData.Products[0].Target : null);
391339
var lifecycle = (input.OutputProducts?.Count > 0 ? input.OutputProducts[0].Lifecycle : null)
392340
?? (bundleData.Products.Count > 0 ? bundleData.Products[0].Lifecycle?.ToStringFast(true) : null);
393341
var owner = input.Owner ?? "elastic";
394342
var repo = input.Repo ?? (bundleData.Products.Count > 0 ? bundleData.Products[0].ProductId : null) ?? "unknown";
395343

396-
var substitutedDescription = BundleDescriptionSubstitution.SubstitutePlaceholders(
397-
input.Description, version, lifecycle, owner, repo);
398-
399-
bundleData = bundleData with { Description = substitutedDescription };
344+
try
345+
{
346+
var substitutedDescription = BundleDescriptionSubstitution.SubstitutePlaceholders(
347+
input.Description, version, lifecycle, owner, repo, validateResolvable: true);
348+
bundleData = bundleData with { Description = substitutedDescription };
349+
}
350+
catch (InvalidOperationException ex)
351+
{
352+
collector.EmitError(string.Empty, $"Description placeholder substitution failed: {ex.Message}");
353+
return false;
354+
}
400355
}
401356

402357
// Write bundle file
@@ -674,7 +629,6 @@ private bool ValidateInput(IDiagnosticsCollector collector, BundleChangelogsArgu
674629

675630
private static bool ValidatePlaceholderUsage(IDiagnosticsCollector collector, BundleChangelogsArguments input)
676631
{
677-
// Only validate in option-based mode (profile mode has separate validation)
678632
if (!string.IsNullOrEmpty(input.Profile))
679633
return true;
680634

‎src/services/Elastic.Changelog/Rendering/Asciidoc/ChangelogAsciidocRenderer.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ public async Task RenderAsciidoc(ChangelogRenderContext context, Cancel ctx)
3333
_ = sb.AppendLine();
3434

3535
// Add description if present
36-
if (!string.IsNullOrEmpty(context.Description))
36+
if (!string.IsNullOrEmpty(context.BundleDescription))
3737
{
38-
_ = sb.AppendLine(context.Description);
38+
_ = sb.AppendLine(context.BundleDescription);
3939
_ = sb.AppendLine();
4040
}
4141

‎src/services/Elastic.Changelog/Rendering/ChangelogRenderContext.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public record ChangelogRenderContext
2727
public required Dictionary<ChangelogEntry, bool> EntryToHideLinks { get; init; }
2828
public ChangelogConfiguration? Configuration { get; init; }
2929
/// <summary>
30-
/// Optional description for the changelog. Only set when there's a single bundle with a description (MVP approach).
30+
/// Optional bundle-level introductory description. Only set when there's a single bundle with a description (MVP approach).
3131
/// </summary>
32-
public string? Description { get; init; }
32+
public string? BundleDescription { get; init; }
3333
}

‎src/services/Elastic.Changelog/Rendering/ChangelogRenderingService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ private static ChangelogRenderContext BuildRenderContext(
308308
EntryToOwner = entryToOwner,
309309
EntryToHideLinks = entryToHideLinks,
310310
Configuration = config,
311-
Description = description
311+
BundleDescription = description
312312
};
313313
}
314314

‎src/services/Elastic.Changelog/Rendering/Markdown/IndexMarkdownRenderer.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ public override async Task RenderAsync(ChangelogRenderContext context, Cancel ct
5353
_ = sb.AppendLine(InvariantCulture, $"## {context.Title} [{context.Repo}-release-notes-{context.TitleSlug}]");
5454

5555
// Add description if present
56-
if (!string.IsNullOrEmpty(context.Description))
56+
if (!string.IsNullOrEmpty(context.BundleDescription))
5757
{
5858
_ = sb.AppendLine();
59-
_ = sb.AppendLine(context.Description);
59+
_ = sb.AppendLine(context.BundleDescription);
6060
}
6161

6262
if (otherLinks.Count > 0)

‎src/tooling/docs-builder/Commands/ChangelogCommand.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,8 @@ public async Task<int> Bundle(
626626
forbidden.Add("--config");
627627
if (!string.IsNullOrWhiteSpace(directory))
628628
forbidden.Add("--directory");
629+
if (!string.IsNullOrWhiteSpace(description))
630+
forbidden.Add("--description");
629631

630632
if (forbidden.Count > 0)
631633
{

‎tests/Elastic.Changelog.Tests/Changelogs/BundleChangelogsTests.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5972,7 +5972,7 @@ public async Task BundleChangelogs_OptionModeWithPlaceholdersButNoOutputProducts
59725972
Directory = _changelogDir,
59735973
All = true,
59745974
Output = outputPath,
5975-
Description = "Release includes {version} with {lifecycle} features from {owner}/{repo}" // Has placeholders but no --output-products
5975+
Description = "Release includes {version} with {lifecycle} features from {owner}/{repo}"
59765976
};
59775977

59785978
// Act
@@ -6022,16 +6022,15 @@ public async Task BundleChangelogs_OptionModeWithPlaceholdersAndOutputProducts_S
60226022
[Fact]
60236023
public async Task BundleChangelogs_OptionModeWithConfigDescriptionAndPlaceholders_ReturnsError()
60246024
{
6025-
// Arrange - test validation with description that could come from config (simulated via CLI)
6025+
// Arrange - config-provided description with placeholders but no --output-products
60266026
CreateSampleChangelogs();
60276027
var outputPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "bundle.yaml");
60286028
var input = new BundleChangelogsArguments
60296029
{
60306030
Directory = _changelogDir,
60316031
All = true,
60326032
Output = outputPath,
6033-
Description = "Version {version} includes {lifecycle} updates from {owner}/{repo}" // Simulate config-provided description
6034-
// No OutputProducts - should fail validation
6033+
Description = "Version {version} includes {lifecycle} updates from {owner}/{repo}"
60356034
};
60366035

60376036
// Act

0 commit comments

Comments
 (0)