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

fix(hz): base type error and multiple service error for client #510

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

FGYFFFF
Copy link
Contributor

@FGYFFFF FGYFFFF commented Jan 4, 2023

What type of PR is this?

fix

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.

(Optional) Translate the PR title into Chinese.

修复 hz 生成的 client 代码在使用基本类型作为入参时的错误,以及定义多 service 时出现的错误

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

en: Fix a bug in hz-generated client code when using basic types as input, and when defining multiple services
zh(optional):
修复 hz 生成的 client 代码在使用基本类型作为入参时的错误,以及定义多 service 时出现的错误

Which issue(s) this PR fixes:

@FGYFFFF FGYFFFF requested review from a team as code owners January 4, 2023 07:03
@FGYFFFF FGYFFFF requested a review from li-jin-gou January 4, 2023 07:06
@codecov
Copy link

codecov bot commented Jan 4, 2023

Codecov Report

Base: 69.34% // Head: 69.42% // Increases project coverage by +0.07% 🎉

Coverage data is based on head (b641961) compared to base (81332e6).
Patch has no changes to coverable lines.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #510      +/-   ##
===========================================
+ Coverage    69.34%   69.42%   +0.07%     
===========================================
  Files           93       93              
  Lines         8874     8874              
===========================================
+ Hits          6154     6161       +7     
+ Misses        2355     2348       -7     
  Partials       365      365              
Impacted Files Coverage Δ
pkg/common/compress/compress.go 83.80% <0.00%> (+2.85%) ⬆️
pkg/common/timer/timer.go 80.95% <0.00%> (+19.04%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Duslia Duslia merged commit 36682f8 into cloudwego:develop Jan 4, 2023
@@ -142,7 +142,8 @@ func (pkgGen *HttpPackageGenerator) Generate(pkg *HttpPackage) error {

if pkgGen.CmdType == meta.CmdClient {
clientDir := pkgGen.IdlClientDir
clientDir = util.SubDir(clientDir, "hertz")
// support multiple service, so disable "clientDir/hertz"
// clientDir = util.SubDir(clientDir, "hertz")
Copy link
Member

Choose a reason for hiding this comment

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

Why not just delete this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do it in another pr, this one has been merged~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done~, in pr

@@ -43,7 +43,8 @@ type ClientFile struct {

func (pkgGen *HttpPackageGenerator) genClient(pkg *HttpPackage, clientDir string) error {
for _, s := range pkg.Services {
hertzClientPath := filepath.Join(clientDir, "hertz_client.go")
cliDir := util.SubDir(clientDir, util.ToSnakeCase(s.Name))
hertzClientPath := filepath.Join(cliDir, "hertz_client.go")
Copy link
Member

Choose a reason for hiding this comment

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

Add a const for hertz_client.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done~, in pr

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.

4 participants