-
-
Notifications
You must be signed in to change notification settings - Fork 128
Branding: support page_background with gradient #99
Branding: support page_background with gradient #99
Conversation
@alexkappa I added some test for the custom Marshalling/Unmarshalling logic. |
I rebased and force pushed to fix a conflict in |
@alexkappa Can you take a look at this? I would love to add support for this to the terraform-provider. |
Hi @apricote, sure thing. Sorry for the delay. I'll give it a look today. |
management/branding.go
Outdated
var data interface{} = struct { | ||
Primary *string `json:"primary,omitempty"` | ||
}{ | ||
Primary: bc.Primary, | ||
} |
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.
This structure is a little uneasy on the eyes, could we define an actual type for it internal to the function instead? Perhaps something similar to user identities:
https://github.com/go-auth0/auth0/blob/master/management/user.go#L105-L109
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.
This might be more helpful: https://gist.github.com/alexkappa/4b541a712dc06c047a38b005178978b5#file-fleet-go-L53-L91
I wrote about this here: https://medium.com/@alexkappa/json-polymorphism-in-go-4cade1e58ed1
management/branding.go
Outdated
Primary: bc.Primary, | ||
jsonPageBackgroundGradient: &jsonPageBackgroundGradient{ | ||
PageBackgroundGradient: bc.PageBackgroundGradient, | ||
}, |
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.
Could this instead be cast?
jsonPageBackgroundGradient: (*jsonPageBackgroundGradient)(bc.PageBackgroundGradient)
Co-authored-by: Alex Kalyvitis <alex.kalyvitis@gmail.com>
@alexkappa Thanks for your review. I updated my code and re-added the unit tests for the Marshalling/Unmarshalling that got lost when i force-pushed the last time. |
Thank you @apricote, much appreciated! I've merged this in. |
Closes #94