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

(feature): go generator supports whitelabelling #2953

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

dsinghvi
Copy link
Member

No description provided.

@dsinghvi dsinghvi marked this pull request as ready for review February 12, 2024 20:57
@dsinghvi dsinghvi requested a review from amckinney February 12, 2024 20:57
@dsinghvi
Copy link
Member Author

@amckinney do you think the go generator would benefit from passing structs around instead of long params? feels easy to make mistakes

@@ -783,6 +795,7 @@ func newClientTestFile(
"client/client_test.go",
"client",
baseImportPath,
false,
Copy link
Member Author

Choose a reason for hiding this comment

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

false for the test file for now but we'll want to plumb it in there as well

@dsinghvi
Copy link
Member Author

Merging due to time sensitivity but can address comments in a FLUP @amckinney

@dsinghvi dsinghvi merged commit a06c798 into main Feb 12, 2024
16 checks passed
@dsinghvi dsinghvi deleted the dsinghvi/support-whitelabel branch February 12, 2024 21:00
@amckinney
Copy link
Member

@amckinney do you think the go generator would benefit from passing structs around instead of long params? feels easy to make mistakes

In many places we prefer structs, but creating an excessive number of params structs anytime we have a large list of parameters can be cumbersome in Go - it requires a separate structure for every function that requires it. I usually tend to reserve parameter structs for the public-most, external-facing APIs, and use in-line parameters for internal functions.

With that said, there are probably some good candidates where a struct makes sense for internal functions (given the sheer number of parameters involved), but it's equally as easy to forget to include a parameter and the default value will be used (unlike other languages like Typescript that enforce every parameter is set).

I'll take a look at this again later and see what we can do to improve the most important use cases.

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.

2 participants