Refinement cleanup #20
Conversation
janstenpickle
force-pushed
the
refinement-cleanup
branch
from
f966868 to
c35d6e3
Compare
janstenpickle
force-pushed
the
refinement-cleanup
branch
from
aa3f5c1 to
8efd46d
Compare
| with Order[Name] | ||
| with Show[Name] { | ||
| override def hash(x: Name): Int = Hash[String].hash(x.value) | ||
| private val invalidNames: Set[String] = Set("quantile", "le", "name") |
There was a problem hiding this comment.
Histograms require that no user defined label is le and summaries require the same for quantile. This raises the question: should we have metric specifc label names, just as we do for metric names?
There was a problem hiding this comment.
Yeah, I think if we can get this cleanly into the DSL (like it is for names) then definitely 👍
If we can't then what I imagine you're doing now (runtime exception) is fine.
There was a problem hiding this comment.
This isn't actually a runtime execption... this is more a question about whether, like with metric names, we should have metric specific labels.
Speaking with @TimWSpence we think it's probably a good idea to go with metric specific for consitency's sake.
There was a problem hiding this comment.
I've just had a go at wiring this through and it is a bit of a pain. Probably not worth the effort considering there are only two labels that we want to prevent peoplefrom using...
Remove some of the boilerplate in refined types
Fixes #18