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
Improvements to generator of conversion methods. #7354
Conversation
@lavalamp you suggested adding a test to checking that functions registered via AddGeneratedConversionFunctions() weren't manually modified. I completely agree this is a good idea but I started thinking what's the best way to do it. This will be even harder as soon as we change those functions to be named and calling them one from another since they will need to be defined outside "AddGeneratedConversionFunctions()" method. Any suggestions on wht is the best way to do it? Do we want to explicitly compare the file as text with what would be generated? Thanks! |
608c60f
to
922555f
Compare
@wojtek-t Yeah, I think auto-generated stuff has to be clearly demarcated in files such that that test can just be a direct comparison. This is also important for humans editing things, so they know to edit the source and not the autogenerated stuff. |
return err | ||
} | ||
if err := writeLine(w, indent+1, "return err\n"); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out http://golang.org/pkg/go/format/#Source for auto-formatting of this code. Then you could ignore tabbing.
if err := writeLine(w, indent, ifStmt); err != nil { | ||
return err | ||
} | ||
newFormat := "out.%s = new(%s)\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe if out.%s == nil { out.%s = new(%s) } \n
?
LGTM -- comments are optional and can be addressed in a followup |
Improvements to generator of conversion methods.
Part of #6800
cc @smarterclayton @lavalamp