This repository was archived by the owner on Oct 15, 2019. It is now read-only.
[codec] Fully support abstract struct types.#280
Closed
jsirois wants to merge 1 commit intofacebookarchive:masterfrom
Closed
[codec] Fully support abstract struct types.#280jsirois wants to merge 1 commit intofacebookarchive:masterfrom
jsirois wants to merge 1 commit intofacebookarchive:masterfrom
Conversation
This addresses: facebookarchive#279 Commit 243fa41 removed the 'structs must be final' check which paved the way for builders to produce (hidden) implementations of abstract structs. Although a pure abstract struct class can be used today, a struct interface cannot due to remaining assumptions in the `ThriftCodecByteCodeGenerator` that the struct type is a class. This change fixes up the `ThriftCodecByteCodeGenerator` to handle structs of interface type and adds tests to cover both the abstract class and interface struct builder cases. Test Plan: Unit tests included. Also, works in a spike for Apache Aurora where swift is being explored as a dual REST/thrift interface generator: https://issues.apache.org/jira/browse/AURORA-987 Reviewers: @andrewcox, @jesboat, @alandau
Contributor
|
FYI - I have a working implementation of this in a PR as well that you may be interested in, in particular it has more comprehensive consistency checks, and I've been using it in production for a while. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
This addresses: #279
Commit 243fa41 removed the 'structs must be final' check which paved
the way for builders to produce (hidden) implementations of abstract
structs. Although a pure abstract struct class can be used today, a
struct interface cannot due to remaining assumptions in the
ThriftCodecByteCodeGeneratorthat the struct type is a class.This change fixes up the
ThriftCodecByteCodeGeneratorto handlestructs of interface type and adds tests to cover both the abstract
class and interface struct builder cases.
Test Plan: Unit tests included. Also, works in a spike for Apache Aurora
where swift is being explored as a dual REST/thrift interface generator:
https://issues.apache.org/jira/browse/AURORA-987
Reviewers: @andrewcox, @jesboat, @alandau