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

Add config_yaml samples #711

Merged
merged 3 commits into from
Mar 14, 2024
Merged

Add config_yaml samples #711

merged 3 commits into from
Mar 14, 2024

Conversation

YarBor
Copy link

@YarBor YarBor commented Mar 11, 2024

Add config_yaml samples , use new api to load config.yaml #697

Signed-off-by: YarBor <yarbor.ww@gmail.com>
Copy link
Member

@FinalT FinalT left a comment

Choose a reason for hiding this comment

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

Thanks! A few comments.

config_yaml/README.md Outdated Show resolved Hide resolved
config_yaml/README_zh.md Outdated Show resolved Hide resolved
Signed-off-by: YarBor <yarbor.ww@gmail.com>
Copy link
Member

@FinalT FinalT left a comment

Choose a reason for hiding this comment

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

LGTM.


### 3.3 Show

Start the server first and then the client. You can observe that the client prints `ConfigTest successfully` and the configuration is loaded and the call is successful.
Copy link
Contributor

Choose a reason for hiding this comment

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

这种中式英文实在难以看懂,建议你找个 GPT 如文心一言翻译下也成哦

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for reminding

Signed-off-by: YarBor <yarbor.ww@gmail.com>
@AlexStocks
Copy link
Contributor

@YarBor 处理完给我留个消息,我再 review 下

@YarBor
Copy link
Author

YarBor commented Mar 12, 2024

@YarBor 处理完给我留个消息,我再 review 下

I think it is done @AlexStocks

Copy link
Contributor

@chickenlj chickenlj left a comment

Choose a reason for hiding this comment

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

LGTM.

@chickenlj chickenlj merged commit e4ca19a into apache:main Mar 14, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants