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: add yaml-config support for trimmer #131

Merged
merged 20 commits into from
Oct 24, 2023

Conversation

tksky1
Copy link
Contributor

@tksky1 tksky1 commented Oct 9, 2023

Description

Add yaml-config support for -m and -p option for trimmer. When using thriftgo/kitex to call trimmer as a feature, these options can also be toggled by the yaml-config in the same directory of target IDL file.

For example:

  • test.thrift
  • trim_config.yaml

By adding configurations below to trim_config.yaml, you'll able to toggle these settings whenever you trim test.thrift:

methods:
  - "TestService.func1"
  - "TestService.func3"
preserve: false

this works the same way as input params.

Motivation and Context

When using trimmer as a feature by thriftgo/kitex, using methods-specified trimming or disable preservation was impossible. By adding yaml-config support, these options can also be set through yaml files.

@tksky1 tksky1 requested review from a team as code owners October 9, 2023 13:04
@HeyJavaBean HeyJavaBean changed the title feat: add yaml-config support for trimmer WIP: feat: add yaml-config support for trimmer Oct 16, 2023
generator/golang/backend.go Outdated Show resolved Hide resolved
tool/trimmer/main.go Outdated Show resolved Hide resolved
@HeyJavaBean HeyJavaBean changed the title WIP: feat: add yaml-config support for trimmer feat: add yaml-config support for trimmer Oct 24, 2023
HeyJavaBean
HeyJavaBean previously approved these changes Oct 24, 2023
@tksky1 tksky1 dismissed HeyJavaBean’s stale review October 24, 2023 07:30

The merge-base changed after approval.

@HeyJavaBean HeyJavaBean enabled auto-merge (squash) October 24, 2023 11:15
HeyJavaBean
HeyJavaBean previously approved these changes Oct 24, 2023
@tksky1 tksky1 dismissed HeyJavaBean’s stale review October 24, 2023 11:16

The merge-base changed after approval.

HeyJavaBean
HeyJavaBean previously approved these changes Oct 24, 2023
@tksky1 tksky1 dismissed HeyJavaBean’s stale review October 24, 2023 11:25

The merge-base changed after approval.

@HeyJavaBean HeyJavaBean merged commit 5325945 into cloudwego:main Oct 24, 2023
3 checks passed
HeyJavaBean added a commit that referenced this pull request Feb 21, 2024
Co-authored-by: Li2CO3 <liyun.339@bytedance.com>
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.

None yet

3 participants