Skip to content

Conversation

Shane32
Copy link
Owner

@Shane32 Shane32 commented Oct 1, 2025

Summary by CodeRabbit

  • Refactor
    • Streamlined PNG export path for QR code images, removing legacy platform-specific handling for more consistent behavior across environments.
  • Chores
    • Simplified internal data-writing logic to reduce complexity and improve maintainability, with no changes to the public API.

@Shane32 Shane32 self-assigned this Oct 1, 2025
Copy link

coderabbitai bot commented Oct 1, 2025

📝 Walkthrough

Walkthrough

The WriteScanlines method in QRCoder/PngByteQRCode.cs removes a .NET 3.5 conditional branch and always writes compressed IDAT data using idatStream.WriteTo(_stream), eliminating CopyTo usage and idatStream.Position resets. No public APIs were changed.

Changes

Cohort / File(s) Summary
PNG IDAT write path
QRCoder/PngByteQRCode.cs
Simplified IDAT emission: removed NET35 conditional; unconditionally uses idatStream.WriteTo(_stream); removed idatStream.Position reset and CopyTo path. No public API changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant W as WriteScanlines
    participant D as DeflateStream
    participant I as idatStream (MemoryStream)
    participant S as _stream (output)

    Note over W,I: Old flow (pre-change)
    W->>D: Compress scanlines into IDAT
    D-->>I: Write compressed bytes
    W->>I: Reset Position = 0
    alt NET35
        W->>S: I.WriteTo(S)
    else non-NET35
        W->>S: I.CopyTo(S)
    end
Loading
sequenceDiagram
    autonumber
    participant W as WriteScanlines
    participant D as DeflateStream
    participant I as idatStream (MemoryStream)
    participant S as _stream (output)

    rect rgba(220,240,255,0.5)
    Note over W,I: New flow (post-change)
    end
    W->>D: Compress scanlines into IDAT
    D-->>I: Write compressed bytes
    Note over W,I: No Position reset
    W->>S: I.WriteTo(S)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Simplify WriteScanlines” accurately and concisely describes the main change of removing the conditional branch in WriteScanlines and focuses on the core simplification, making it clear to reviewers what this pull request addresses.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch simplify_write_scanlines

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 446aa0c and 4b2acaf.

📒 Files selected for processing (1)
  • QRCoder/PngByteQRCode.cs (0 hunks)
💤 Files with no reviewable changes (1)
  • QRCoder/PngByteQRCode.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

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.

// Compressed data.
idatStream.Position = 0;
#if NET35
idatStream.WriteTo(_stream);
Copy link
Owner Author

Choose a reason for hiding this comment

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

WriteTo writes the entire MemoryStream (disregarding position) to the target stream, and is a supported method across all .NET variations.

@Shane32 Shane32 requested a review from gfoidl October 1, 2025 15:52
@Shane32 Shane32 merged commit e355a87 into master Oct 1, 2025
12 checks passed
@Shane32 Shane32 deleted the simplify_write_scanlines branch October 1, 2025 22:19
@Shane32 Shane32 added this to the 1.7.0 milestone Oct 2, 2025
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