Duration is missing in some ChangeCommentedEvent and ChangeReviewedEvent

[leonid-deriv]

Probably 50% of the events above do not have the duration field. It is not clear why. Is this a bug or a feature?

[morucci]

Hi, the duration field is optional and according to the code only merged or closed PRs got the duration field. https://github.com/change-metrics/monocle/blob/master/src/Lentille/GitHub/Utils.hs#L183

[leonid-deriv]

is this really correct? does it matter for the events above that PR is closed or merged. Or do you mean that duration will be added to these events when PR is closed?

[morucci]

Yes when the PR is merged or closed then the duration is added to the Change object and related Event objects.

[leonid-deriv]

Maybe I am wrong but it will be less complex to add duration at the time we create events. Plus, as I mentioned above, there are benefits in having this information even if PR is not merged yet - what is the downside ?. I can be wrong of course :)

[morucci]

Yes we could set the duration for not merged/closed Changes however the duration won't be accurate most of the time if the Change is not merged or closed. That sounds more logical to let the data consumer (eg. the Monocle UI) figures out the (change in-progress) duration at query time. Could the scripted fields help for your use case https://www.elastic.co/guide/en/elasticsearch/reference/current/search-fields.html#script-fields ? What is the use case ?

[leonid-deriv]

The metric is PR pickup time - when there is a first comment. Probably I can filter by only merge/closed PRs but I feel this it does not matter what will happen with PR. Also I think the time should be consistent because I understand we measure the time between PR creation and comment and PR creation time will be the same irrespective of whether you merged it or not. No?

[morucci]

The duration on the Event object is the same as the duration on a Change object.

Perhaps what you would like is a two new fields:

• delay_for_first_comment
• delay_for_first_review

These new fields could be added to Change objects ?

[leonid-deriv]

For me one field will be enough - it does not matter whether it was a comment or review (not sure why GitHub has two different events)