FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: prevent large payload bytes print when compression is ineffective by yiweichi · Pull Request #70 · scroll-tech/da-codec · GitHub

/ da-codec Public

feat: prevent large payload bytes print when compression is ineffective - #70

Merged
yiweichi merged 2 commits into
mainfrom
feat-improve-log
Nov 28, 2025
ConversationCommits (2)ChecksFiles changed
Merged

feat: prevent large payload bytes print when compression is ineffective#70
yiweichi merged 2 commits into
mainfrom
feat-improve-log

Conversation

yiweichi commented Nov 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Purpose or design rationale of this PR

This PR improves the log, removing large payload bytes print when compression is ineffective. The payload is not meaningful for debugging, should be enough to barely print the length of original/compressed payload bytes.

Corresponding issue: #68

PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
  • ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
  • docs: Documentation-only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test: Adding missing tests or correcting existing tests

Breaking change label

Does this PR have the breaking-change label?

  • No, this PR is not a breaking change
  • Yes

Summary by CodeRabbit

  • Chores
    • Improved diagnostic logging by replacing detailed payload data representations with concise length metrics in compression compatibility checks for enhanced log readability and reduced verbosity.

✏️ Tip: You can customize this high-level summary in your review settings.

coderabbitai Bot commented Nov 28, 2025 •
edited
Loading

Copy link
Copy Markdown

Walkthrough

This PR modifies warning logs in the checkCompressedDataCompatibility function across codec versions v7, v8, and v9. The changes replace hex-encoded payload dumps with payload length metrics (payloadLen and compressedPayloadLen) in two warning log statements per file. No functional logic or return values are affected.

Changes

Cohort / File(s) Summary
Logging updates in codec versions
encoding/codecv7.go, encoding/codecv8.go, encoding/codecv9.go		 Modified warning logs in checkCompressedDataCompatibility to report payload lengths instead of hex-encoded payload dumps. Two log statements per file updated: one on compression compatibility failure, one on length check failure. Purely diagnostic changes with no behavioral impact.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Homogeneous changes applied consistently across three identical files
  • Non-functional logging-only modifications
  • Straightforward field substitution with no logic changes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • Thegaram
  • jonastheis
  • noel2004

Poem

🐰 Logs were thick with hex-encoded dreams,
But now they flow with length-based streams,
Smaller files, cleaner sight,
A rabbit's trim delight! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: preventing large payload bytes from being printed in logs when compression is ineffective, which aligns with the changeset modifications across three codec files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description covers the purpose/rationale and includes linked issue, but the PR title itself is not provided separately—only the conventional commits template is shown.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-improve-log

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.

Thegaram linked an issue Nov 28, 2025 that may be closed by this pull request
Excessive logging of full payload bytes in compression warning bloats log files #68
Closed
yiweichi merged commit 5492978 into main Nov 28, 2025
4 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Thegaram Thegaram approved these changes
+1 more reviewer
jonastheis jonastheis approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Excessive logging of full payload bytes in compression warning bloats log files

3 participants

Back | FazBrowse Home | New Git URL