-
Notifications
You must be signed in to change notification settings - Fork 351
Add notice to S2 about MinLZ #1065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I have taken the experiences from this library and created a backwards compatible compression package called MinLZ. That package will seamlessly decode S2 content, making the transition from this package fairly trivial. There are many improvements to pretty much all aspects of S2 since we have "broken free" of the Snappy format specification. You can read a writeup on [Design and Improvements over S2](https://gist.github.com/klauspost/a25b66198cdbdf7b5b224f670c894ed5). The only aspect not covered is custom dictionary encoding. While I do intend to fix errors in this package, I do not expect to make significant improvements, since I consider MinLZ a better basis for going forward. See https://github.com/minio/minlz for all details.
📝 WalkthroughWalkthroughThe pull request updates the Changes
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
s2/README.md (3)
3-3: Hyphenate Compound AdjectiveIn the sentence "I have taken the experiences from this library and created a backwards compatible compression package called MinLZ," the compound adjective should be hyphenated. Consider changing "backwards compatible" to "backwards-compatible" for improved clarity and adherence to style guidelines.
-I have taken the experiences from this library and created a backwards compatible compression package called MinLZ. +I have taken the experiences from this library and created a backwards-compatible compression package called MinLZ.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...riences from this library and created a backwards compatible compression package called MinLZ. That...(EN_COMPOUND_ADJECTIVE_INTERNAL)
7-7: Refine Informal LanguageThe phrase "pretty much" in "There are many improvements to pretty much all aspects of S2 since we have 'broken free' of the Snappy format specification." is somewhat informal. Consider removing or replacing it (e.g., "There are many improvements to all aspects of S2...") for a more professional tone.
🧰 Tools
🪛 LanguageTool
[style] ~7-~7: The phrase ‘pretty much’ can be informal. To strengthen your writing, consider removing it or replacing it with an adverb.
Context: ...rivial. There are many improvements to pretty much all aspects of S2 since we have "broken...(PRETTY_MUCH)
13-13: Convert Bare URL to Markdown LinkLine 13 uses a bare URL ("See https://github.com/minio/minlz for all details."). Converting it to a Markdown link will enhance readability and consistency with the rest of the document.
-See https://github.com/minio/minlz for all details. +See [MinLZ details](https://github.com/minio/minlz) for all details.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
s2/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
s2/README.md
[uncategorized] ~3-~3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...riences from this library and created a backwards compatible compression package called MinLZ. That...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~7-~7: The phrase ‘pretty much’ can be informal. To strengthen your writing, consider removing it or replacing it with an adverb.
Context: ...rivial. There are many improvements to pretty much all aspects of S2 since we have "broken...
(PRETTY_MUCH)
🪛 markdownlint-cli2 (0.17.2)
s2/README.md
13-13: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: fuzz-other (nounsafe)
- GitHub Check: fuzz-other ("noasm,nounsafe")
- GitHub Check: build (1.22.x, windows-latest)
- GitHub Check: build (1.24.x, ubuntu-latest)
- GitHub Check: build (1.23.x, macos-latest)
- GitHub Check: build (1.24.x, macos-latest)
- GitHub Check: build (1.24.x, windows-latest)
- GitHub Check: build (1.23.x, ubuntu-latest)
- GitHub Check: build-special
- GitHub Check: build (1.23.x, windows-latest)
- GitHub Check: fuzz-zstd (nounsafe)
- GitHub Check: build (1.22.x, macos-latest)
- GitHub Check: fuzz-s2 ("noasm,nounsafe")
- GitHub Check: fuzz-s2 (nounsafe)
- GitHub Check: build (1.22.x, ubuntu-latest)
🔇 Additional comments (1)
s2/README.md (1)
1-2: Section Header Clarity and ConsistencyThe new "MinLZ" header is clear and prominent. Please ensure its style (spacing, capitalization, etc.) aligns with other major section headers throughout the document.
This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [github.com/klauspost/compress](https://github.com/klauspost/compress) | `v1.18.0` -> `v1.18.1` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>klauspost/compress (github.com/klauspost/compress)</summary> ### [`v1.18.1`](https://github.com/klauspost/compress/releases/tag/v1.18.1) [Compare Source](klauspost/compress@v1.18.0...v1.18.1) #### What's Changed - zstd: Fix incorrect buffer size in dictionary encodes by [@​klauspost](https://github.com/klauspost) in [#​1059](klauspost/compress#1059) - s2: check for cap, not len of buffer in EncodeBetter/Best by [@​vdarulis](https://github.com/vdarulis) in [#​1080](klauspost/compress#1080) - zstd: Add simple zstd EncodeTo/DecodeTo functions by [@​klauspost](https://github.com/klauspost) in [#​1079](klauspost/compress#1079) - zlib: Avoiding extra allocation in zlib.reader.Reset by [@​travelpolicy](https://github.com/travelpolicy) in [#​1086](klauspost/compress#1086) - gzhttp: remove redundant err check in zstdReader by [@​ryanfowler](https://github.com/ryanfowler) in [#​1090](klauspost/compress#1090) - Run modernize. Deprecate Go 1.22 by [@​klauspost](https://github.com/klauspost) in [#​1095](klauspost/compress#1095) - flate: Simplify matchlen by [@​klauspost](https://github.com/klauspost) in [#​1101](klauspost/compress#1101) - flate: Add examples by [@​klauspost](https://github.com/klauspost) in [#​1102](klauspost/compress#1102) - flate: Use exact sizes for huffman tables by [@​klauspost](https://github.com/klauspost) in [#​1103](klauspost/compress#1103) - flate: Faster load+store by [@​klauspost](https://github.com/klauspost) in [#​1104](klauspost/compress#1104) - Add notice to S2 about MinLZ by [@​klauspost](https://github.com/klauspost) in [#​1065](klauspost/compress#1065) #### New Contributors - [@​wooffie](https://github.com/wooffie) made their first contribution in [#​1069](klauspost/compress#1069) - [@​vdarulis](https://github.com/vdarulis) made their first contribution in [#​1080](klauspost/compress#1080) - [@​travelpolicy](https://github.com/travelpolicy) made their first contribution in [#​1086](klauspost/compress#1086) - [@​ryanfowler](https://github.com/ryanfowler) made their first contribution in [#​1090](klauspost/compress#1090) **Full Changelog**: <klauspost/compress@v1.18.0...v1.18.1> </details> --- ### Configuration 📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xNTIuOSIsInVwZGF0ZWRJblZlciI6IjQxLjE1Mi45IiwidGFyZ2V0QnJhbmNoIjoiZm9yZ2VqbyIsImxhYmVscyI6WyJkZXBlbmRlbmN5LXVwZ3JhZGUiLCJ0ZXN0L25vdC1uZWVkZWQiXX0=--> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9786 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: Renovate Bot <forgejo-renovate-action@forgejo.org> Co-committed-by: Renovate Bot <forgejo-renovate-action@forgejo.org>
I have taken the experiences from this library and created a backwards compatible compression package called MinLZ.
That package will seamlessly decode S2 content, making the transition from this package fairly trivial.
There are many improvements to pretty much all aspects of S2 since we have "broken free" of the Snappy format specification. You can read a writeup on Design and Improvements over S2.
The only aspect not covered is custom dictionary encoding. While I do intend to fix errors in this package, I do not expect to make significant improvements, since I consider MinLZ a better basis for going forward.
See https://github.com/minio/minlz for all details.
Summary by CodeRabbit