Skip to content

Commit a5e1c17

Browse files
Mpdreamzclaude
andcommitted
Address CodeRabbit review comments
CsvReader: remove GetType().Name == "MockFileSystem" type check — the check never matched after ScopedFileSystem wrapping was introduced, causing Sep to read from the real filesystem via FromFile() instead of going through the scoped IFileSystem. Always use IFileSystem.File.ReadAllText + Sep.FromText so all filesystem access (real, scoped, or mock) goes through the abstraction. ChangelogCommand: four ChangelogConfigurationLoader instantiations were still passing new System.IO.Abstractions.FileSystem() directly, bypassing the scoped _fileSystem field. Replace with _fileSystem. ChangelogRenderingService: service writes output files (CreateDirectory, WriteAllTextAsync) so its default should be FileSystemFactory.RealWrite not RealRead. RealWrite has the same scope but correctly excludes .git write access. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 8406bba commit a5e1c17

3 files changed

Lines changed: 13 additions & 32 deletions

File tree

src/Elastic.Markdown/Myst/Directives/CsvInclude/CsvReader.cs

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,35 +22,16 @@ private static IEnumerable<string[]> ReadWithSep(string filePath, string separat
2222
var separatorChar = separator == "," ? ',' : separator[0];
2323
var spec = Sep.New(separatorChar);
2424

25-
// Sep works with actual file paths, not virtual file systems
26-
// For testing with MockFileSystem, we'll read content first
27-
if (fileSystem.GetType().Name == "MockFileSystem")
28-
{
29-
var content = fileSystem.File.ReadAllText(filePath);
30-
using var reader = spec.Reader(o => o with { HasHeader = false, Unescape = true }).FromText(content);
25+
// Always read via IFileSystem so that scoped and mock file systems are respected.
26+
var content = fileSystem.File.ReadAllText(filePath);
27+
using var reader = spec.Reader(o => o with { HasHeader = false, Unescape = true }).FromText(content);
3128

32-
foreach (var row in reader)
33-
{
34-
var rowData = new string[row.ColCount];
35-
for (var i = 0; i < row.ColCount; i++)
36-
rowData[i] = row[i].ToString();
37-
yield return rowData;
38-
}
39-
}
40-
else
29+
foreach (var row in reader)
4130
{
42-
using var reader = spec.Reader(o => o with { HasHeader = false, Unescape = true }).FromFile(filePath);
43-
44-
foreach (var row in reader)
45-
{
46-
var rowData = new string[row.ColCount];
47-
for (var i = 0; i < row.ColCount; i++)
48-
{
49-
rowData[i] = row[i].ToString();
50-
}
51-
yield return rowData;
52-
}
31+
var rowData = new string[row.ColCount];
32+
for (var i = 0; i < row.ColCount; i++)
33+
rowData[i] = row[i].ToString();
34+
yield return rowData;
5335
}
5436
}
55-
5637
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public class ChangelogRenderingService(
7070
) : IService
7171
{
7272
private readonly ILogger _logger = logFactory.CreateLogger<ChangelogRenderingService>();
73-
private readonly ScopedFileSystem _fileSystem = fileSystem ?? FileSystemFactory.RealRead;
73+
private readonly ScopedFileSystem _fileSystem = fileSystem ?? FileSystemFactory.RealWrite;
7474

7575
public async Task<bool> RenderChangelogs(
7676
IDiagnosticsCollector collector,

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ public async Task<int> Create(
288288
// Load changelog config and apply fallbacks for all modes.
289289
// Precedence: CLI option > bundle section in changelog.yml > built-in default.
290290
// This applies to --prs, --issues, and --release-version alike.
291-
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, new System.IO.Abstractions.FileSystem())
291+
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem)
292292
.LoadChangelogConfiguration(collector, config, ctx);
293293
var resolvedRepo = !string.IsNullOrWhiteSpace(repo) ? repo : bundleConfig?.Bundle?.Repo;
294294
var resolvedOwner = owner ?? bundleConfig?.Bundle?.Owner ?? "elastic";
@@ -542,7 +542,7 @@ public async Task<int> Bundle(
542542
}
543543

544544
// Precedence: --repo CLI > bundle.repo config; --owner CLI > bundle.owner config > "elastic"
545-
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, new System.IO.Abstractions.FileSystem())
545+
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem)
546546
.LoadChangelogConfiguration(collector, config, ctx);
547547
var resolvedRepo = !string.IsNullOrWhiteSpace(repo) ? repo : bundleConfig?.Bundle?.Repo;
548548
var resolvedOwner = owner ?? bundleConfig?.Bundle?.Owner ?? "elastic";
@@ -845,7 +845,7 @@ public async Task<int> Remove(
845845
}
846846

847847
// Precedence: --repo CLI > bundle.repo config; --owner CLI > bundle.owner config > "elastic"
848-
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, new System.IO.Abstractions.FileSystem())
848+
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem)
849849
.LoadChangelogConfiguration(collector, config, ctx);
850850
var resolvedRepo = !string.IsNullOrWhiteSpace(repo) ? repo : bundleConfig?.Bundle?.Repo;
851851
var resolvedOwner = owner ?? bundleConfig?.Bundle?.Owner ?? "elastic";
@@ -1104,7 +1104,7 @@ public async Task<int> GitHubRelease(
11041104
await using var serviceInvoker = new ServiceInvoker(collector);
11051105

11061106
// --output CLI > bundle.directory config > ./changelogs (service default)
1107-
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, new System.IO.Abstractions.FileSystem())
1107+
var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem)
11081108
.LoadChangelogConfiguration(collector, config, ctx);
11091109
var resolvedOutput = !string.IsNullOrWhiteSpace(output) ? output : bundleConfig?.Bundle?.Directory;
11101110

0 commit comments

Comments
 (0)