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 "gf gen pb" api and ctrl not working well. #3076

Merged
merged 11 commits into from
Nov 8, 2023
Merged

fix "gf gen pb" api and ctrl not working well. #3076

merged 11 commits into from
Nov 8, 2023

Conversation

joy999
Copy link
Contributor

@joy999 joy999 commented Oct 22, 2023

No description provided.

@joy999 joy999 changed the title fix api and ctrl not working well. fix "gf gen pb" api and ctrl not working well. Oct 22, 2023
cmd/gf/internal/cmd/genpb/genpb.go Outdated Show resolved Hide resolved
@hailaz
Copy link
Member

hailaz commented Nov 2, 2023

现在只改了个单词错误?

@hailaz
Copy link
Member

hailaz commented Nov 2, 2023

那就顺便把这些改了 @joy999
image
image

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Only one word mistake has been corrected now?

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Then change these by the way @joy999
image
image

@joy999
Copy link
Contributor Author

joy999 commented Nov 2, 2023

这个PR还没完成哦~ 还要修改gcmd才算OK。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


This PR is not completed yet~ You still need to modify gcmd to be OK.

@hailaz
Copy link
Member

hailaz commented Nov 2, 2023

这个PR还没完成哦~ 还要修改gcmd才算OK。

name:"api" 是name这个tag失效?

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


This PR is not completed yet~ You still need to modify gcmd to be OK.

name:"api" is the name tag invalid?

@hailaz hailaz added the wip label Nov 3, 2023
@joy999
Copy link
Contributor Author

joy999 commented Nov 4, 2023

之前的逻辑只会判定field.Name(),所以不会判定 tag: name,现在给加了如果tag: name存在,则优先使用它进行判定。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


The previous logic would only determine field.Name(), so tag: name would not be determined. Now it is added that if tag: name exists, it will be used first for determination.

@joy999 joy999 requested a review from gqcn November 4, 2023 15:49
@gqcn
Copy link
Member

gqcn commented Nov 6, 2023

之前的逻辑只会判定field.Name(),所以不会判定 tag: name,现在给加了如果tag: name存在,则优先使用它进行判定。

我看了,改得挺好的,就是缺少对应的单测,麻烦加上对应的单测呢❤️。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


The previous logic would only determine field.Name(), so tag: name would not be determined. Now it is added that if tag: name exists, it will be used first for determination.

I took a look, and the changes are pretty good, but the corresponding single test is missing. Could you please add the corresponding single test ❤️.

@hailaz hailaz added done This issue is done, which may be release in next version. and removed wip labels Nov 7, 2023
@gqcn
Copy link
Member

gqcn commented Nov 8, 2023

@joy999 @hailaz You guys are awesome! ❤️

@gqcn gqcn merged commit 0b407c5 into master Nov 8, 2023
40 checks passed
@gqcn gqcn deleted the fix/gf_gen_pb branch November 8, 2023 13:26
@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@joy999 @hailaz You guys are awesome! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
done This issue is done, which may be release in next version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants