Skip to content

refactor: Move internal polyfills to Internals namespace#250

Merged
skarllot merged 2 commits intomainfrom
feat/use-internals-ns
Mar 18, 2026
Merged

refactor: Move internal polyfills to Internals namespace#250
skarllot merged 2 commits intomainfrom
feat/use-internals-ns

Conversation

@skarllot
Copy link
Copy Markdown
Owner

@skarllot skarllot commented Mar 18, 2026

Summary

  • Moves InternalSpanLineEnumeratorSpanLineEnumerator and StringBuilderMemory into the Raiqub.Generators.InterpolationCodeWriter.Internals namespace
  • Adds SpanExtensions with EnumerateLines() extension method, unifying the netstandard2.0 and newer-target code paths
  • Removes #if NETSTANDARD2_0 guards in SourceTextWriter.Write.cs now that polyfills are always available via the Internals namespace

Test plan

  • Build succeeds with no warnings-as-errors
  • All existing unit tests pass (InternalSpanLineEnumeratorTest, StringBuilderMemoryTest, and writer tests)
  • CSharp10 test project compiles correctly after removing the InternalPolyfill compile include

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Reorganized internal code structure into a dedicated namespace for better maintainability.
    • Simplified line-handling implementation by removing legacy platform-specific code paths.
    • Converted internal utility methods to extension methods for improved code ergonomics.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 18, 2026

Warning

Rate limit exceeded

@skarllot has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 36 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a4bc0ca8-20b5-4418-a1ac-42ae5532238d

📥 Commits

Reviewing files that changed from the base of the PR and between 221325a and ceb5364.

📒 Files selected for processing (1)
  • tests/InterpolationCodeWriter.Tests/SpanLineEnumeratorTest.cs
📝 Walkthrough

Walkthrough

This PR reorganizes internal utilities into a dedicated namespace by moving types from Raiqub.Generators.InterpolationCodeWriter to Raiqub.Generators.InterpolationCodeWriter.Internals, renaming InternalSpanLineEnumerator to SpanLineEnumerator, adding a new SpanExtensions class, converting StringBuilderMemory.Append to an extension method, and removing NETSTANDARD2_0 conditional compilation logic.

Changes

Cohort / File(s) Summary
Namespace and Project Configuration
src/InternalPolyfill/InternalPolyfill.csproj
Updated RootNamespace to reflect the new Internals namespace hierarchy.
Internal Utilities Reorganization
src/InternalPolyfill/SpanLineEnumerator.cs, src/InternalPolyfill/StringBuilderMemory.cs, src/InternalPolyfill/SpanExtensions.cs
Moved types to Internals namespace; renamed InternalSpanLineEnumerator to SpanLineEnumerator; added SpanExtensions with EnumerateLines extension method; converted StringBuilderMemory.Append to extension method.
Consumer Code Updates
src/InterpolationCodeWriter/SourceTextWriter.Write.cs
Added Internals namespace import; removed NETSTANDARD2_0 conditional compilation; simplified line enumeration to use new extension methods unconditionally.
Test Updates
tests/InterpolationCodeWriter.Tests/InternalSpanLineEnumeratorTest.cs, tests/InterpolationCodeWriter.Tests/StringBuilderMemoryTest.cs, tests/InterpolationCodeWriter.CSharp10.Tests/InterpolationCodeWriter.CSharp10.Tests.csproj
Updated test references to use SpanLineEnumerator and Internals namespace; removed InternalPolyfill source file inclusion from test project.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • fgodoy-lepaho

Poem

🐰 In burrows of code, we tidy and organize,
Internals nested deep, a cleaner sunrise,
Span and StringBuilder dance in harmony,
No more conditions, just simplicity!
The garden of Generators now gleams so bright,
Refactored with care, everything right!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main refactoring work: moving internal polyfills to an Internals namespace.
Description check ✅ Passed The PR description contains all required template sections with detailed explanations of changes, test plan confirmation, and build verification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/use-internals-ns
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 18, 2026

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.88%. Comparing base (beb42b7) to head (ceb5364).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/InternalPolyfill/SpanExtensions.cs 0.00% 3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (25.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #250      +/-   ##
==========================================
- Coverage   89.08%   88.88%   -0.21%     
==========================================
  Files          18       19       +1     
  Lines        1329     1332       +3     
  Branches      101      101              
==========================================
  Hits         1184     1184              
- Misses        101      104       +3     
  Partials       44       44              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fgodoy-lepaho
fgodoy-lepaho previously approved these changes Mar 18, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/InterpolationCodeWriter.Tests/InternalSpanLineEnumeratorTest.cs (1)

1-2: LGTM!

Test updates correctly reference the renamed SpanLineEnumerator type from the Internals namespace. The test class name InternalSpanLineEnumeratorTest remains as a historical artifact—consider renaming it to SpanLineEnumeratorTest in a follow-up for consistency, but this is not blocking.

Also applies to: 90-90, 116-116

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/InterpolationCodeWriter.Tests/InternalSpanLineEnumeratorTest.cs` around
lines 1 - 2, The test currently references the renamed SpanLineEnumerator type
from Internals and passes, but the test class name
InternalSpanLineEnumeratorTest is now inconsistent; to clean up, rename the test
class InternalSpanLineEnumeratorTest to SpanLineEnumeratorTest and update any
references (test class declaration and constructor/method attribute usages) so
they match the new name, ensuring SpanLineEnumerator from
Raiqub.Generators.InterpolationCodeWriter.Internals remains imported and used
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/InterpolationCodeWriter.Tests/InternalSpanLineEnumeratorTest.cs`:
- Around line 1-2: The test currently references the renamed SpanLineEnumerator
type from Internals and passes, but the test class name
InternalSpanLineEnumeratorTest is now inconsistent; to clean up, rename the test
class InternalSpanLineEnumeratorTest to SpanLineEnumeratorTest and update any
references (test class declaration and constructor/method attribute usages) so
they match the new name, ensuring SpanLineEnumerator from
Raiqub.Generators.InterpolationCodeWriter.Internals remains imported and used
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 08435335-065f-42ed-a3ab-4a1e51068ed8

📥 Commits

Reviewing files that changed from the base of the PR and between beb42b7 and 221325a.

📒 Files selected for processing (8)
  • src/InternalPolyfill/InternalPolyfill.csproj
  • src/InternalPolyfill/SpanExtensions.cs
  • src/InternalPolyfill/SpanLineEnumerator.cs
  • src/InternalPolyfill/StringBuilderMemory.cs
  • src/InterpolationCodeWriter/SourceTextWriter.Write.cs
  • tests/InterpolationCodeWriter.CSharp10.Tests/InterpolationCodeWriter.CSharp10.Tests.csproj
  • tests/InterpolationCodeWriter.Tests/InternalSpanLineEnumeratorTest.cs
  • tests/InterpolationCodeWriter.Tests/StringBuilderMemoryTest.cs
💤 Files with no reviewable changes (1)
  • tests/InterpolationCodeWriter.CSharp10.Tests/InterpolationCodeWriter.CSharp10.Tests.csproj

@skarllot skarllot merged commit 0fe09ff into main Mar 18, 2026
5 checks passed
@skarllot skarllot deleted the feat/use-internals-ns branch March 18, 2026 00:13
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