refactor: Let EncodingFactory be an instantiable class#607
Closed
tanjialiang wants to merge 1 commit intofacebookincubator:mainfrom
Closed
refactor: Let EncodingFactory be an instantiable class#607tanjialiang wants to merge 1 commit intofacebookincubator:mainfrom
tanjialiang wants to merge 1 commit intofacebookincubator:mainfrom
Conversation
|
@tanjialiang has exported this pull request. If you are a Meta employee, you can view the originating Diff in D97367204. |
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Mar 24, 2026
…acebookincubator#607) Summary: Introduce `encoding::Decoder` class that bundles a decode function with `Encoding::Options` into a single object. This replaces the pattern of threading a bare `std::function` encoding factory through multiple layers (`NimbleData`, `ChunkedDecoder`, `NimbleParams`). Named constructors: - `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode` - `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode` This is a pure refactoring with no behavioral change. It simplifies the selective reader code by replacing 10+ line inline lambda ternaries with concise `encoding::Decoder::create()` / `createLegacy()` calls, and prepares the codebase for threading additional options (like varint row counts) without touching every callsite. The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a separate BUCK target (`encoding_decoder`) to avoid a circular dependency between `encodings` and `legacy/encodings`. Differential Revision: D97367204
454e69d to
68f7ba1
Compare
xiaoxmeng
approved these changes
Mar 25, 2026
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Mar 25, 2026
Summary: Pull Request resolved: facebookincubator#607 Introduce `encoding::Decoder` class that bundles a decode function with `Encoding::Options` into a single object. This replaces the pattern of threading a bare `std::function` encoding factory through multiple layers (`NimbleData`, `ChunkedDecoder`, `NimbleParams`). Named constructors: - `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode` - `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode` This is a pure refactoring with no behavioral change. It simplifies the selective reader code by replacing 10+ line inline lambda ternaries with concise `encoding::Decoder::create()` / `createLegacy()` calls, and prepares the codebase for threading additional options (like varint row counts) without touching every callsite. The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a separate BUCK target (`encoding_decoder`) to avoid a circular dependency between `encodings` and `legacy/encodings`. Reviewed By: xiaoxmeng Differential Revision: D97367204
68f7ba1 to
d37b376
Compare
d37b376 to
4c9e449
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Mar 26, 2026
Summary: Pull Request resolved: facebookincubator#607 Introduce `encoding::Decoder` class that bundles a decode function with `Encoding::Options` into a single object. This replaces the pattern of threading a bare `std::function` encoding factory through multiple layers (`NimbleData`, `ChunkedDecoder`, `NimbleParams`). Named constructors: - `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode` - `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode` This is a pure refactoring with no behavioral change. It simplifies the selective reader code by replacing 10+ line inline lambda ternaries with concise `encoding::Decoder::create()` / `createLegacy()` calls, and prepares the codebase for threading additional options (like varint row counts) without touching every callsite. The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a separate BUCK target (`encoding_decoder`) to avoid a circular dependency between `encodings` and `legacy/encodings`. Reviewed By: xiaoxmeng Differential Revision: D97367204
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Mar 26, 2026
Summary: Pull Request resolved: facebookincubator#607 Introduce `encoding::Decoder` class that bundles a decode function with `Encoding::Options` into a single object. This replaces the pattern of threading a bare `std::function` encoding factory through multiple layers (`NimbleData`, `ChunkedDecoder`, `NimbleParams`). Named constructors: - `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode` - `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode` This is a pure refactoring with no behavioral change. It simplifies the selective reader code by replacing 10+ line inline lambda ternaries with concise `encoding::Decoder::create()` / `createLegacy()` calls, and prepares the codebase for threading additional options (like varint row counts) without touching every callsite. The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a separate BUCK target (`encoding_decoder`) to avoid a circular dependency between `encodings` and `legacy/encodings`. Reviewed By: xiaoxmeng Differential Revision: D97367204
4c9e449 to
3ebf708
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Mar 26, 2026
Summary: Pull Request resolved: facebookincubator#607 Introduce `encoding::Decoder` class that bundles a decode function with `Encoding::Options` into a single object. This replaces the pattern of threading a bare `std::function` encoding factory through multiple layers (`NimbleData`, `ChunkedDecoder`, `NimbleParams`). Named constructors: - `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode` - `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode` This is a pure refactoring with no behavioral change. It simplifies the selective reader code by replacing 10+ line inline lambda ternaries with concise `encoding::Decoder::create()` / `createLegacy()` calls, and prepares the codebase for threading additional options (like varint row counts) without touching every callsite. The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a separate BUCK target (`encoding_decoder`) to avoid a circular dependency between `encodings` and `legacy/encodings`. Reviewed By: xiaoxmeng Differential Revision: D97367204
3ebf708 to
1a2f139
Compare
tanjialiang
added a commit
to tanjialiang/nimble
that referenced
this pull request
Mar 26, 2026
Summary: Pull Request resolved: facebookincubator#607 Introduce `encoding::Decoder` class that bundles a decode function with `Encoding::Options` into a single object. This replaces the pattern of threading a bare `std::function` encoding factory through multiple layers (`NimbleData`, `ChunkedDecoder`, `NimbleParams`). Named constructors: - `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode` - `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode` This is a pure refactoring with no behavioral change. It simplifies the selective reader code by replacing 10+ line inline lambda ternaries with concise `encoding::Decoder::create()` / `createLegacy()` calls, and prepares the codebase for threading additional options (like varint row counts) without touching every callsite. The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a separate BUCK target (`encoding_decoder`) to avoid a circular dependency between `encodings` and `legacy/encodings`. Reviewed By: xiaoxmeng Differential Revision: D97367204
1a2f139 to
457d011
Compare
Summary: Pull Request resolved: facebookincubator#607 Introduce `encoding::Decoder` class that bundles a decode function with `Encoding::Options` into a single object. This replaces the pattern of threading a bare `std::function` encoding factory through multiple layers (`NimbleData`, `ChunkedDecoder`, `NimbleParams`). Named constructors: - `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode` - `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode` This is a pure refactoring with no behavioral change. It simplifies the selective reader code by replacing 10+ line inline lambda ternaries with concise `encoding::Decoder::create()` / `createLegacy()` calls, and prepares the codebase for threading additional options (like varint row counts) without touching every callsite. The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a separate BUCK target (`encoding_decoder`) to avoid a circular dependency between `encodings` and `legacy/encodings`. Reviewed By: xiaoxmeng Differential Revision: D97367204
457d011 to
2677e08
Compare
|
This pull request has been merged in 44a28d9. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Introduce
encoding::Decoderclass that bundles a decode function withEncoding::Optionsinto a single object. This replaces the pattern ofthreading a bare
std::functionencoding factory through multiple layers(
NimbleData,ChunkedDecoder,NimbleParams).Named constructors:
encoding::Decoder::create(Options)— usesEncodingFactory::decodeencoding::Decoder::createLegacy(Options)— useslegacy::EncodingFactory::decodeThis is a pure refactoring with no behavioral change. It simplifies the
selective reader code by replacing 10+ line inline lambda ternaries with
concise
encoding::Decoder::create()/createLegacy()calls, andprepares the codebase for threading additional options (like varint row
counts) without touching every callsite.
The
Decoderclass lives innamespace facebook::nimble::encodingand in aseparate BUCK target (
encoding_decoder) to avoid a circular dependencybetween
encodingsandlegacy/encodings.Reviewed By: xiaoxmeng
Differential Revision: D97367204