You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
thema.Library should be renamed to thema.Context. Its methods should also be converted to requiring a pointer, thereby requiring all existing function signatures to switch to *thema.Context.
I originally named it Library because i thought this was a nice nod to how we basically load a bunch of CUE "functions" in from the thema CUE package, then call them from the various Go methods within thema. And i wanted it to be a value, not a pointer, because i was hoping to make the zero value useful, similar to e.g. sync.Mutex.
However:
Library contains a cue.Context, and shuffling that thing around has ended up feeling like the main job of Library when actually using the Thema go package.
I'm not thrilled with calling it thema.Context because it sorta muddies the water by not having cancellation capabilities. But my real objection there is with how Go stdlib already muddied those waters by conflating variable bags together with cancellation. Asking people to learn a new term, Library, is a heavy lift for just resolving that ambiguity. Better to just fall in line with CUE itself and use thema.Context.
Implementing thema within Grafana has made it pretty clear that allowing an explicit nil is preferable for centralizing use of a single shared context. Adding nil to the accepted value space of a function signature allow the caller to make it clear that they are not specifying a value, and expecting the func impl to grab the central one instead. This could be accomplished without pointers by passing the zero value - thema.Library{} - but that feels like undocumented magic; expressing the absence of a value is, unfortunately, one of the main use cases for nil pointers in Go.
Thus: thema.Library -> *thema.Context.
The text was updated successfully, but these errors were encountered:
Changed my mind - *thema.Runtime is better. Seeing a thema.Context and a cue.Context is just too much stutter, especially given that i think that the name is misleading in the first place.
There's a functional reason this is important, too - without the thing being a pointer type, we can't use a mutex to even try to guard against CUE current lack of thread safety due to data races from arising from the reference counter (see: cue-lang/cue#652 (comment))
Changes the name of thema.Library to thema.Runtime, makes it a pointer,
and tries to put mutex guards around any kind of concurrency that could
cause the CUE internals to blow up due to its naive reference counter
implementation.
Fixes#67
thema.Library
should be renamed tothema.Context
. Its methods should also be converted to requiring a pointer, thereby requiring all existing function signatures to switch to*thema.Context
.I originally named it
Library
because i thought this was a nice nod to how we basically load a bunch of CUE "functions" in from thethema
CUE package, then call them from the various Go methods within thema. And i wanted it to be a value, not a pointer, because i was hoping to make the zero value useful, similar to e.g.sync.Mutex
.However:
Library
contains acue.Context
, and shuffling that thing around has ended up feeling like the main job ofLibrary
when actually using the Thema go package.thema.Context
because it sorta muddies the water by not having cancellation capabilities. But my real objection there is with how Go stdlib already muddied those waters by conflating variable bags together with cancellation. Asking people to learn a new term,Library
, is a heavy lift for just resolving that ambiguity. Better to just fall in line with CUE itself and usethema.Context
.nil
is preferable for centralizing use of a single shared context. Addingnil
to the accepted value space of a function signature allow the caller to make it clear that they are not specifying a value, and expecting the func impl to grab the central one instead. This could be accomplished without pointers by passing the zero value -thema.Library{}
- but that feels like undocumented magic; expressing the absence of a value is, unfortunately, one of the main use cases for nil pointers in Go.Thus:
thema.Library
->*thema.Context
.The text was updated successfully, but these errors were encountered: