search

exoscale / egoscale

exoscale golang bindings
https://pkg.go.dev/github.com/exoscale/egoscale/v3
Apache License 2.0
31 stars 15 forks source link

v3: helpers for dealing with pointers #591

Closed kobajagi closed 11 months ago

shortcut-integration[bot] commented 11 months ago

This pull request has been linked to Shortcut Story #71886: Helpers for dealing with pointers.

kobajagi commented 11 months ago

LGTM, I have only a suggestion. Perhaps it would be worth it to put these helpers into a separate package? helpers or utils as package names come to mind. Calling a helper function as v3.FromString() seems odd to me.

I actually did that initially (there is already a utils package) but then moved the file to the root. I was thinking about how using it from utils requires another package import and our root namespace is quite clean so they won't clutter it (thus marginally better ux). I'm not strongly opinionated either way so whatever is a general sentiment I'll go with it.

sauterp commented 11 months ago

LGTM, I have only a suggestion. Perhaps it would be worth it to put these helpers into a separate package? helpers or utils as package names come to mind. Calling a helper function as v3.FromString() seems odd to me.

I actually did that initially (there is already a utils package) but then moved the file to the root. I was thinking about how using it from utils requires another package import and our root namespace is quite clean so they won't clutter it (thus marginally better ux). I'm not strongly opinionated either way so whatever is a general sentiment I'll go with it.

Makes sense, in a way this similar to how aws SDK is organized, where the have aws.String() without needing an extra import https://github.com/aws/aws-sdk-go-v2/blob/v1.20.1/aws/to_ptr.go#L45