Skip to content
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

feat:(thrift) nested thrift type constructor API #41

Merged
merged 8 commits into from
Apr 29, 2024

Conversation

AsterDY
Copy link
Collaborator

@AsterDY AsterDY commented Mar 15, 2024

What type of PR is this?

feat

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.
  • Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. User docs repo

(Optional) Translate the PR title into Chinese.

  1. 提供thrift嵌套类型的构造函数
  • NewNodeList
  • NewNodeSet
  • NewNodeMap
  • NewNodeStruct
  • NewNodeAny
  1. 修复 NewTypedNode 未设置实际内容的问题

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en:
zh(optional):

(Optional) Which issue(s) this PR fixes:

(optional) The PR that updates user documentation:

thrift/binary.go Outdated Show resolved Hide resolved
thrift/binary.go Outdated Show resolved Hide resolved
}
}
return STRUCT, p.WriteFieldStop()
default:

Choose a reason for hiding this comment

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

Suggestion: opensource task for the community to encode arbitrary type with reflection, which supports zero-length slice/map.

Choose a reason for hiding this comment

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

Another approach may be to generate all possible combinations of the types based on go generate with a template to achieve best performance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO

thrift/generic/node.go Outdated Show resolved Hide resolved
thrift/generic/node.go Outdated Show resolved Hide resolved
@AsterDY AsterDY force-pushed the feat/thrift_node_constructor branch from 780a3fd to 70e85df Compare April 29, 2024 03:32
@AsterDY AsterDY force-pushed the feat/thrift_node_constructor branch from 70e85df to 24742ca Compare April 29, 2024 03:41
@AsterDY AsterDY merged commit 4e6336a into main Apr 29, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants