Rename metric instrument "Handles" to "Bound Instruments" #70
Conversation
jmacd
requested review from
AloisReitbauer,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
reyang,
SergeyKanzhelev,
songy23,
tedsuo,
tigrannajaryan and
yurishkuro
as code owners
|
|
||
| This is a simple renaming. All uses of "handle" will be replaced by | ||
| "bound instrument" in the specification. All uses of the `GetHandle` | ||
| method become `Bind`. |
There was a problem hiding this comment.
How about GetBoundInstrument() as it makes it more clear that this method returns something. (like GetHandle() .
Bind() may be wrongly interpreted as an action or something one can use to manually bind instrument and labelset as in like meter.Bind(labelset, instrument)
There was a problem hiding this comment.
It is an action, it creates a new object, bound instrument.
There was a problem hiding this comment.
It is an action, but @cijothomas is pointing out that "Bind" doesn't imply that it returns something.
I could be convinced to move the Bind() method to the Meter as you have done, and to use a GetBoundX method on the instrument, but this method will just call Bind() (which will return something) and I'm not sure it's an improvement in clarity.
…etry#70) * Add Bound Instruments OTEP * Add status
…etry#70) * Add Bound Instruments OTEP * Add status
…etry#70) * Add Bound Instruments OTEP * Add status
…etry/oteps#70) * Add Bound Instruments OTEP * Add status
The topic of "Handle" being too general purpose has come up repeatedly. Each time this proposed renaming is discussed, people seem to agree it's an improvement.