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

Be feat ignore method #79

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

exfly
Copy link

@exfly exfly commented Aug 24, 2023

grpc server stub has mustEmbedUnimplementedScannerServiceServer method, gowrap will failed unexported method, ignore it.

Copy link
Owner

@hexdigest hexdigest left a comment

Choose a reason for hiding this comment

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

Hey @exfly

This is a good option, but please take a look at my comment.

@@ -56,6 +57,7 @@ func NewGenerateCommand(l remoteTemplateLoader) *GenerateCommand {
"run `gowrap template list` for details")
fs.Var(&gc.vars, "v", "a key-value pair to parametrize the template,\narguments without an equal sign are treated as a bool values,\ni.e. -v foo=bar -v disableChecks")
fs.StringVar(&gc.localPrefix, "l", "", "put imports beginning with this string after 3rd-party packages; comma-separated list")
fs.StringVar(&gc.ignoreMethod, "ignore_method", "", "ignore method")
Copy link
Owner

Choose a reason for hiding this comment

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

Let's make it boolean flag. Because I don't think people will be picking methods to ignore, most likely these will be just all unexported methods to ignore.

fs.BoolVar(&gc.ignoreUnexported, "u", false, "ignore unexported methods")

@grachevko
Copy link

any news?

@grachevko
Copy link

grachevko commented Apr 11, 2024

@hexdigest if gowrap no support unexported methods, why not just skip it by default without flags?

In my opinion flag to skip make sense in case gowrap generate unexported methods too

@hexdigest
Copy link
Owner

@hexdigest if gowrap no support unexported methods, why not just skip it by default without flags?

In my opinion flag to skip make sense in case gowrap generate unexported methods too

It's not that gowrap does not support unexported methods its about where the generated wrapper is placed. Because there's clearly a use case for generating wrapper with unexported methods if the wrapper is placed within the same package where the interface with unexported method is defined and being called and/or type asserted.

Also conceptually these are two different interfaces and the user has to have a clear understanding on what's going on so the unexported method can't just be ignored, there should be an explicit flag for that.

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.

3 participants