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 #287: Adds -pubtypes, -pubfuncs, and -jsontag CLI flags #313

Closed
wants to merge 1 commit into from

Conversation

chirino
Copy link

@chirino chirino commented May 5, 2024

  • -pubtypes make the generated types public
  • -pubfuncs makes the generated functions public
  • -jsontag add a json:”...” field tag

* -pubtypes make the generated types public
* -pubfuncs makes the generated functions public
* -jsontag add a `json:”...”` field tag

Signed-off-by: Hiram Chirino <[email protected]>
@fmoor
Copy link
Member

fmoor commented May 6, 2024

Awesome, thanks for this!

Would you add test cases for this? Copy the cmd/edgeql-go/testdata/no-args directory and rename once for each of the new options. Adjust the directory content for the behavior with the option being tested. Then add a test case for each new directory here.

It would be great if the new options were added to the usage example in the docs.

I'm wondering if the -jsontag should perhaps be more general. Maybe it could be -tags and accept name+strategy pairs like json:snakecase. This would allow adding multiple tags and being able to choose the name transformation strategy. I think it would be nice to split this into its own PR. I don't want the design discussion for this to hold up the other two options.

@chirino
Copy link
Author

chirino commented Jun 12, 2024

Feel free to make any changes you need. I thought I would send you this contribution to get you started. But I'm fine using a fork.

@fmoor
Copy link
Member

fmoor commented Jul 9, 2024

Closing in favor of #317 and #318.

@fmoor fmoor closed this Jul 9, 2024
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.

2 participants