change-metrics / monocle

Monocle helps teams and individual to better organize daily duties and to detect anomalies in the way changes are produced and reviewed.
https://demo.changemetrics.io/
GNU Affero General Public License v3.0
362 stars 56 forks source link

Incorrect commit time in ChangeCommitPushedEvent #1074

Closed leonid-deriv closed 8 months ago

leonid-deriv commented 8 months ago

both create_at and on_created_at times are times when the PullRequest is created. Probably on_created_at should be a time when a commit is pushed (obviously this time is available in git and it is visible in GitHub) ... or there should be another field which has a time of a commit.

Any chance this will be fixed? Thank you.

morucci commented 8 months ago

For a Change object, created_at is the creation date of the PullRequest, on_created_at does not seems to be set in that case. For any ChangeEvent objects, created_at is the creation date of the PullRequest's event, on_created_at is set at the creation date of the related PullRequest.

The Change object have a commits field with all related PR's commits.

leonid-deriv commented 8 months ago

WE have commits but there are no times for commits. For me, the obvious event to have the correct commit time is in the ChangeCommitPushedEvent the "forced" version of it.

on_created_at does not seems to be set in that case. - does it mean we can set it to commit time?

morucci commented 8 months ago

The ChangeCommitPushedEvent uses the pushedDate of https://docs.github.com/en/graphql/reference/objects#commit for the created_at. However, the ChangeCommitForcePushedEvent is computed from the https://docs.github.com/en/graphql/reference/objects#headrefforcepushedevent PR's event. And the created_at is the date of the event, so perhaps it does not match the last commit's date. I might instead use the afterCommit's pushedDate.

... but I've just seen that pushedDate is deprecated. So committedDate might be more accurate for both ChangeCommitPushedEvent and ChangeCommitForcePushedEvent.

leonid-deriv commented 8 months ago

commited date should be fine