Skip to content

refactor(csharp): consolidate CloudFetch memory buffer default to single source of truth#294

Merged
eric-wang-1990 merged 2 commits intomainfrom
fix/csharp-cloudfetch-memory-buffer-default
Mar 5, 2026
Merged

refactor(csharp): consolidate CloudFetch memory buffer default to single source of truth#294
eric-wang-1990 merged 2 commits intomainfrom
fix/csharp-cloudfetch-memory-buffer-default

Conversation

@eric-wang-1990
Copy link
Collaborator

@eric-wang-1990 eric-wang-1990 commented Mar 4, 2026

Summary

  • CloudFetchConfiguration defaulted to 100MB while CloudFetchMemoryBufferManager had its own duplicate constant defaulting to 200MB
  • The factory always passes config.MemoryBufferSizeMB to the manager, making the manager's private constant dead code in production — but the two definitions could silently drift apart
  • Aligns CloudFetchConfiguration.DefaultMemoryBufferSizeMB to 200MB (matching the manager's original intent)
  • Removes the duplicate DefaultMemoryBufferSizeMB constant from CloudFetchMemoryBufferManager and replaces it with a reference to CloudFetchConfiguration.DefaultMemoryBufferSizeMB
  • Simplifies constructor parameter from int? maxMemoryMB = null to int maxMemoryMB = CloudFetchConfiguration.DefaultMemoryBufferSizeMB
  • Updates test assertion to reference CloudFetchConfiguration.DefaultMemoryBufferSizeMB instead of a magic number

Test plan

  • Existing CloudFetchMemoryBufferManagerTests pass unchanged
  • No functional behavior change — factory call sites pass config.MemoryBufferSizeMB explicitly and are unaffected

🤖 Generated with Claude Code

CloudFetchMemoryBufferManager defaults to 200MB when no value is
provided, but CloudFetchConfiguration defaulted to 100MB. The factory
always passes config.MemoryBufferSizeMB to the manager, so the effective
default was 100MB — inconsistent with the manager's own constant.

Aligns both to 200MB so the defaults agree regardless of how
CloudFetchMemoryBufferManager is instantiated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iguration

Remove duplicate DefaultMemoryBufferSizeMB constant from CloudFetchMemoryBufferManager
and reference CloudFetchConfiguration.DefaultMemoryBufferSizeMB as the single source
of truth. Also simplify constructor from nullable int? to int with default parameter.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@eric-wang-1990 eric-wang-1990 changed the title fix(csharp): align CloudFetchConfiguration memory default to 200MB refactor(csharp): consolidate CloudFetch memory buffer default to single source of truth Mar 4, 2026
@eric-wang-1990 eric-wang-1990 added this pull request to the merge queue Mar 4, 2026
Merged via the queue into main with commit 53f137a Mar 5, 2026
18 of 19 checks passed
@eric-wang-1990 eric-wang-1990 deleted the fix/csharp-cloudfetch-memory-buffer-default branch March 5, 2026 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants