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 addition info for consul #86

Merged
merged 3 commits into from
Sep 22, 2023

Conversation

L2ncE
Copy link
Member

@L2ncE L2ncE commented Jul 31, 2023

What type of PR is this?

feat

What this PR does / why we need it (English/Chinese):

在之前的 Consul 拓展实现中,直接使用 Hertz 提供的 Info 结构体中的 Tags 作为 Consul 的 Meta,并且 Consul 非常重要的 Tags 也没有进行配置的入口。因此在这里提供了一个新的结构体来为用户方便配置 Tags 与 Meta。

In the previous Consul extension implementation, the Tags field in the Info structure provided by Hertz was directly used as the Meta value of Consul, and the very important Tags field of Consul did not have an entry for configuration.
Therefore, a new structure is provided here to facilitate the configuration of Tags and Meta for users.

Which issue(s) this PR fixes:

cloudwego/hertz#596

@welkeyever
Copy link

Please check the Tests / ut - there are issues in it.

@L2ncE
Copy link
Member Author

L2ncE commented Aug 4, 2023

Please check the Tests / ut - there are issues in it.

It seems to be a Nacos problem, not caused by this PR. Can you rerun it for me, and I will check if there is still a problem.

@GuangmingLuo
Copy link
Contributor

@L2ncE please check the test result.

@L2ncE
Copy link
Member Author

L2ncE commented Aug 25, 2023

@L2ncE please check the test result.

Got it, I'll fix it in my spare time.

@L2ncE
Copy link
Member Author

L2ncE commented Aug 25, 2023

@welkeyever @GuangmingLuo PTAL❤️

@GuangmingLuo
Copy link
Contributor

@li-jin-gou please take a look

Copy link

@welkeyever welkeyever left a comment

Choose a reason for hiding this comment

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

LGTM, thx~

@welkeyever welkeyever merged commit 66c5157 into hertz-contrib:main Sep 22, 2023
@li-jin-gou
Copy link
Collaborator

li-jin-gou commented Dec 28, 2023 via email

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