Fixed large buffer rent #1
Conversation
|
Warning Rate limit exceeded@j-fuxa has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
WalkthroughUpdated EditorConfig with extensive C# style rules and stricter analyzers; added repository metadata; removed the Qodana workflow; changed release publish to GitHub Packages; expanded ChunkDataReadStream and ChunkDataWriteStream with Span- and single-byte stream overrides while preserving read/write semantics. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant R as ChunkDataReadStream
participant U as ChunkFileStream
Note over R,U: New Span-based read path
Caller->>R: Read(Span<byte> buffer)
R->>U: Read(Span<byte> buffer)
U-->>R: bytesRead
R-->>Caller: bytesRead
alt Single-byte read
Caller->>R: ReadByte()
R->>R: allocate byte (Unsafe.SkipInit)
R->>U: Read(Span<byte>(1))
alt bytesRead == 1
R-->>Caller: return 0..255
else EOF
R-->>Caller: -1
end
end
rect rgba(255,230,230,0.5)
Note over R: Write attempts remain invalid
Caller->>R: Write(ReadOnlySpan<byte>) / WriteByte(byte)
R-->>Caller: throw InvalidOperationException
end
sequenceDiagram
actor Caller
participant W as ChunkDataWriteStream
Note over W: Read attempts on write-only stream
Caller->>W: Read(Span<byte>) / ReadByte()
W-->>Caller: throw InvalidOperationException
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
j-fuxa
force-pushed
the
buffer
branch
2 times, most recently
from
d25d512 to
6ad0e7f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/release.yml (1)
25-25: Harden nuget push: add --skip-duplicate (and consider symbol push or GH token).Add --skip-duplicate to avoid failing re-runs on an already-published version. Optional: push symbols (.snupkg) if produced; ensure the token used has write:packages.
- dotnet nuget push artefacts/EventStore.Plugins.${{ steps.get_version.outputs.version }}.nupkg -k ${{ secrets.nugetkey }} -s https://nuget.pkg.github.com/TrogonStack/index.json + dotnet nuget push artefacts/EventStore.Plugins.${{ steps.get_version.outputs.version }}.nupkg -s https://nuget.pkg.github.com/TrogonStack/index.json -k ${{ secrets.nugetkey }} --skip-duplicate +# If you also publish symbols: +# dotnet nuget push artefacts/EventStore.Plugins.${{ steps.get_version.outputs.version }}.snupkg -s https://nuget.pkg.github.com/TrogonStack/index.json -k ${{ secrets.nugetkey }} --skip-duplicateVerification checklist:
- Secret has write:packages scope (or use GITHUB_TOKEN if you prefer first‑party creds).
- Package id/owner matches feed (TrogonStack).
src/EventStore.Plugins/EventStore.Plugins.csproj (2)
25-28: Enable SourceLink and CI-friendly package metadata.Recommend turning on SourceLink and deterministic CI builds so consumers get PDB/source mapping and reproducible packages.
<PropertyGroup> <RepositoryUrl>https://github.com/TrogonStack/TrogonEventStore.Plugins</RepositoryUrl> <RepositoryType>git</RepositoryType> + <PublishRepositoryUrl>true</PublishRepositoryUrl> + <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild> + <Deterministic>true</Deterministic> + <IncludeSymbols>true</IncludeSymbols> + <SymbolPackageFormat>snupkg</SymbolPackageFormat> </PropertyGroup> +<ItemGroup> + <PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" /> +</ItemGroup>
11-11: Whitespace-only diffs.These appear to be blank-line changes only. Consider reverting to reduce diff noise unless intentional.
Also applies to: 24-24, 29-29, 37-37, 41-41
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.editorconfig(3 hunks).github/workflows/qodana.yml(0 hunks).github/workflows/release.yml(1 hunks)src/EventStore.Plugins/EventStore.Plugins.csproj(2 hunks)src/EventStore.Plugins/Transforms/ChunkDataReadStream.cs(2 hunks)src/EventStore.Plugins/Transforms/ChunkDataWriteStream.cs(3 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/qodana.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- src/EventStore.Plugins/Transforms/ChunkDataWriteStream.cs
- src/EventStore.Plugins/Transforms/ChunkDataReadStream.cs
- .editorconfig
🔇 Additional comments (1)
src/EventStore.Plugins/EventStore.Plugins.csproj (1)
25-28: Good addition: repository metadata included in the package.Adding RepositoryUrl/RepositoryType improves package provenance and SourceLink readiness.
j-fuxa
force-pushed
the
buffer
branch
2 times, most recently
from
e870e44 to
b16da4e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.editorconfig (1)
1-1: Use HTTPS for the EditorConfig URL.Prefer the canonical HTTPS link.
-# EditorConfig is awesome: http://EditorConfig.org +# EditorConfig is awesome: https://editorconfig.org
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.editorconfig(3 hunks).github/workflows/qodana.yml(0 hunks).github/workflows/release.yml(2 hunks).gitignore(1 hunks)src/EventStore.Plugins/EventStore.Plugins.csproj(2 hunks)src/EventStore.Plugins/Transforms/ChunkDataReadStream.cs(2 hunks)src/EventStore.Plugins/Transforms/ChunkDataWriteStream.cs(3 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/qodana.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- .gitignore
- .github/workflows/release.yml
- src/EventStore.Plugins/EventStore.Plugins.csproj
- src/EventStore.Plugins/Transforms/ChunkDataWriteStream.cs
- src/EventStore.Plugins/Transforms/ChunkDataReadStream.cs
🔇 Additional comments (2)
.editorconfig (2)
25-28: LangVersion already ≥10 on .NET 8.0—no change needed
All projects target net8.0 with<LangVersion>latest</LangVersion>, satisfying the C# 10+ requirement for IDE0161 enforcement.
14-14: Confirm repo-wide encoding policy; no UTF-8 BOMs detected in .cs files
Runningrg -nU --binary '^\xEF\xBB\xBF' -g '*.cs' -Sproduced no matches, but please manually verify existing C# file encodings to avoid noisy diffs. If none contain a BOM, movecharset = utf-8under[*]in.editorconfigand re-save files consistently.
| # use ValueTasks correctly | ||
| dotnet_diagnostic.CA2012.severity = error | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
CA2012 as error—ensure analyzers are enabled repo-wide.
This rule only triggers if .NET analyzers are enabled. Also expect breakages where ValueTask is incorrectly consumed; consider staging as warning first.
Verify analyzer settings:
🏁 Script executed:
#!/bin/bash
# Look for EnableNETAnalyzers / AnalysisLevel
fd -t f '*.csproj' -x bash -lc 'echo "== {} =="; rg -n "EnableNETAnalyzers|AnalysisLevel|CodeAnalysisTreatWarningsAsErrors|TreatWarningsAsErrors" {} -N || true'
# Also check Directory.Build.props/targets
fd -t f 'Directory.Build.*' -x bash -lc 'echo "== {} =="; cat {}'Length of output: 518
🏁 Script executed:
#!/bin/bash
# Correctly search all .csproj files for analyzer settings
fd --glob '*.csproj' -t f -x bash -lc '
echo "== {} =="
grep -nE "EnableNETAnalyzers|AnalysisLevel|CodeAnalysisTreatWarningsAsErrors|TreatWarningsAsErrors" "{}" || true
'
# List Directory.Build.props and Directory.Build.targets contents
fd --glob 'Directory.Build.*' -t f -x bash -lc '
echo "== {} =="
cat "{}"
'Length of output: 695
Enable .NET analyzers globally before elevating CA2012 to error.
Projects only have <TreatWarningsAsErrors>true</TreatWarningsAsErrors> in individual .csproj files; there’s no <EnableNETAnalyzers> or <AnalysisLevel> defined in a shared Directory.Build.props/targets. Add to Directory.Build.props:
<PropertyGroup>
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisLevel>latest</AnalysisLevel>
</PropertyGroup>Then consider staging dotnet_diagnostic.CA2012.severity = warning before switching to error to minimize initial breakages.
🤖 Prompt for AI Agents
.editorconfig around lines 29-31: the repo sets
dotnet_diagnostic.CA2012.severity = error but .NET analyzers aren’t enabled
globally, so add EnableNETAnalyzers=true and AnalysisLevel=latest to the shared
Directory.Build.props (create or update it at repo root) to ensure analyzers run
consistently across projects; after enabling analyzers, consider changing the
.editorconfig entry to dotnet_diagnostic.CA2012.severity = warning first to
catch and fix issues gradually before promoting it back to error.
5d542e8 to
2f9f274
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.editorconfig (1)
29-31: CA2012 as error: enable analyzers repo-wide or stage to warning.
This setting is a no-op unless .NET analyzers are enabled; making it error without enabling analyzers will not trigger. If enabled later, builds may suddenly fail.Option A (stage change):
-dotnet_diagnostic.CA2012.severity = error +dotnet_diagnostic.CA2012.severity = warningAlso add Directory.Build.props at repo root:
<Project> <PropertyGroup> <EnableNETAnalyzers>true</EnableNETAnalyzers> <AnalysisLevel>latest</AnalysisLevel> </PropertyGroup> </Project>Check current state:
#!/bin/bash # Verify analyzers and treat-as-error settings fd -t f --glob '*.csproj' -x bash -lc ' echo "== {} ==" rg -n "EnableNETAnalyzers|AnalysisLevel|TreatWarningsAsErrors|CodeAnalysisTreatWarningsAsErrors" "{}" -N || true ' fd -t f --glob 'Directory.Build.*' -X bash -lc 'echo "== {} =="; cat "{}"'
🧹 Nitpick comments (1)
.editorconfig (1)
1-1: Use HTTPS and canonical domain in the header comment.
Minor polish; prefer https://editorconfig.org.-# EditorConfig is awesome: http://EditorConfig.org +# EditorConfig is awesome: https://editorconfig.org
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.editorconfig(3 hunks).github/workflows/qodana.yml(0 hunks).github/workflows/release.yml(2 hunks).gitignore(1 hunks)EventStore.Plugins.sln(0 hunks)src/EventStore.Plugins/EventStore.Plugins.csproj(2 hunks)src/EventStore.Plugins/Transforms/ChunkDataReadStream.cs(2 hunks)src/EventStore.Plugins/Transforms/ChunkDataWriteStream.cs(3 hunks)
💤 Files with no reviewable changes (2)
- EventStore.Plugins.sln
- .github/workflows/qodana.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- .gitignore
- src/EventStore.Plugins/Transforms/ChunkDataWriteStream.cs
- src/EventStore.Plugins/EventStore.Plugins.csproj
- .github/workflows/release.yml
- src/EventStore.Plugins/Transforms/ChunkDataReadStream.cs
🔇 Additional comments (2)
.editorconfig (2)
14-14: LGTM: explicit UTF-8 for C# files.
Good default; pairs well with insert_final_newline.
25-28: IDE0161 severity = error is safe to enforce
All projects target .NET 8.0 with LangVersion set to “latest,” so C# 10+ features (including file-scoped namespaces) are supported by every project.
… of Stream.Read(Span<byte>) Removed: qodana.yml Signed-off-by: JOSE FUXA <[email protected]>
j-fuxa
force-pushed
the
master
branch
2 times, most recently
from
1226455 to
49c35af
Compare
Changed: Fixed large buffer rent caused by the default implementatio of Stream.Read(Span)
Removed: qodana.yml