Polishing Observation API by jonatan-ivanov · Pull Request #3425 · micrometer-metrics/micrometer (original) (raw)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss if we want to rename this class.
@shakuzen Did you get some feedback on this?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we rename existing observation to createNotStarted?
Either way, it is better to be unified.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yepp, that's why the let's discuss comment.
I think:

1. If we want to keep the class name as-is: it is better to rename everything to createNotStarted
2. If we want to rename the class to ObservationDocumentation (or similar): it is better to rename everything to observation
3. If we want to keep changes minimal: what I did

I think the best would be # 2 but it is also the most invasive.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ObservationDocumentation better expresses what the interface is for and makes it clear that it is not an Observation itself. The feedback I got so far was that they agreed with that. I think it will be easier to consider that change in a separate pull request, though, so we don't block making the rest of these changes.