Skip to content

Commit

Permalink
feat!: make getTimeToFirstReview use ready for review
Browse files Browse the repository at this point in the history 
Instead of starting the clock for time to first review when the pull
request is created, have it start when the pull request is marked as
ready for review. If the pull request was never in a draft state, then
use the pull request created timestamp as the start of the clock.
  • Loading branch information
@hectcastro
hectcastro committed Apr 11, 2022
1 parent bad5f1e commit f1ddce8
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 34 deletions.
14 changes: 7 additions & 7 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,23 @@ $ gh metrics --owner cli --repo cli
│ 5423 │ 1 │ 41 │ 8 │ 2 │ 1h7m │ 0 │ 2 │ 67h55m │ -- │ 66h44m │
│ 5410 │ 4 │ 10 │ 74 │ 4 │ 12h45m │ 3 │ 4 │ 44h39m │ 26h47m │ 30h41m │
│ 5409 │ 2 │ 113 │ 1 │ 3 │ 11h58m │ 0 │ 3 │ 24h56m │ 7h29m │ 9m │
│ 5408 │ 1 │ 3 │ 5 │ 1 │ 13h25m │ 0 │ 2 │ 18h15m │ -- │ 4h52m │
│ 5408 │ 1 │ 3 │ 5 │ 1 │ 13h15m │ 0 │ 2 │ 18h15m │ -- │ 4h52m │
│ 5406 │ 1 │ 1 │ 0 │ 1 │ 18h5m │ 1 │ 4 │ 18h19m │ -- │ 14m │
│ 5405 │ 1 │ 2 │ 8 │ 1 │ 2h17m │ 0 │ 5 │ 19h0m │ 15h47m │ 16h38m │
│ 5405 │ 1 │ 2 │ 8 │ 1 │ 2h9m │ 0 │ 5 │ 19h0m │ 15h47m │ 16h38m │
│ 5396 │ 1 │ 37 │ 2 │ 2 │ 207h33m │ 0 │ 3 │ 207h40m │ -- │ 6m │
│ 5390 │ 7 │ 183 │ 0 │ 3 │ 65h41m │ 1 │ 4 │ 329h11m │ 23h56m │ 4h29m │
│ 5388 │ 4 │ 8 │ 3 │ 5 │ 89h0m │ 0 │ 4 │ 238h47m │ 149h47m │ 1h39m │
│ 5380 │ 5 │ 54 │ 7 │ 3 │ -- │ 1 │ 6 │ 282h6m │ 277h6m │ -- │
│ 5378 │ 2 │ 58 │ 10 │ 9 │ 184h0m │ 0 │ 3 │ 285h40m │ -- │ 101h36m │
│ 5366 │ 6 │ 101 │ 376 │ 12 │ 23h58m │ 1 │ 2 │ 234h38m │ 195h57m │ 27m │
│ 5380 │ 5 │ 54 │ 7 │ 3 │ 4h58m │ 1 │ 6 │ 282h6m │ 277h6m │ -- │
│ 5378 │ 2 │ 58 │ 10 │ 9 │ 184h0m │ 1 │ 4 │ 285h40m │ -- │ 101h36m │
│ 5366 │ 6 │ 101 │ 376 │ 12 │ 2h8m │ 1 │ 2 │ 234h38m │ 195h57m │ 27m │
└──────┴─────────┴───────────┴───────────┴───────────────┴──────────────────────┴──────────┴──────────────┴───────────────────┴─────────────────────────────┴─────────────────────────┘
```

Or, within a more precise window of time:

```console
$ gh metrics --owner cli --repo cli --start 2022-03-21 --end 2022-03-22
┌──────┬─────────┬───────────┬───────────┬───────────────┬──────────────────────┬──────────┬──────────────┬───────────────────┬─────────────────────────────┬─────────────────────────┐
┌──────┬─────────┬───────────┬───────────┬───────────────┬──────────────────────┌──────┬─────────┬───────────┬───────────┬───────────────┬──────────────────────┬──────────┬──────────────┬───────────────────┬─────────────────────────────┬─────────────────────────┐
│ PR │ COMMITS │ ADDITIONS │ DELETIONS │ CHANGED FILES │ TIME TO FIRST REVIEW │ COMMENTS │ PARTICIPANTS │ FEATURE LEAD TIME │ FIRST REVIEW TO LAST REVIEW │ FIRST APPROVAL TO MERGE │
├──────┼─────────┼───────────┼───────────┼───────────────┼──────────────────────┼──────────┼──────────────┼───────────────────┼─────────────────────────────┼─────────────────────────┤
│ 5339 │ 4 │ 6 │ 3 │ 1 │ 2m │ 0 │ 3 │ 1h12m │ 59m │ 1h9m │
Expand All @@ -63,7 +63,7 @@ PR,Commits,Additions,Deletions,Changed Files,Time to First Review,Comments,Parti

## Metric definitions

- **Time to first review**: The duration from when the pull request was created to when the first review against it was completed.
- **Time to first review**: The duration from when the pull request was created or marked *Ready for review* to when the first review against it was completed.
- **Feature lead time**: The duration from when the first commit contained in the pull request was created to when the pull request was merged.
- **First review to last review**: The duration between the first non-author review and the last approving non-author review ([Background](https://github.com/hectcastro/gh-metrics/issues/13))
- **First approval to merge**: The duration from when the first approval review is given to when the pull request is merged.
Expand Down
35 changes: 21 additions & 14 deletions cmd/graphql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,36 @@ type Commits struct {
Nodes CommitNodes
}

type LatestReviewNodes []struct {
type ReadyForReviewEvent struct {
CreatedAt string
}

type LatestReviews struct {
Nodes LatestReviewNodes
type TimelineItemNodes []struct {
ReadyForReviewEvent ReadyForReviewEvent `graphql:"... on ReadyForReviewEvent"`
}

type TimelineItems struct {
TotalCount int
Nodes TimelineItemNodes
}

type MetricsGQLQuery struct {
Search struct {
Nodes []struct {
PullRequest struct {
Author Author
Additions int
Deletions int
Number int
CreatedAt string
ChangedFiles int
MergedAt string
Participants Participants
Comments Comments
Reviews Reviews `graphql:"reviews(first: 50, states: [APPROVED, CHANGES_REQUESTED, COMMENTED])"`
Commits Commits `graphql:"commits(first: 1)"`
Author Author
Additions int
Deletions int
Number int
CreatedAt string
ChangedFiles int
IsDraft bool
MergedAt string
Participants Participants
Comments Comments
Reviews Reviews `graphql:"reviews(first: 50, states: [APPROVED, CHANGES_REQUESTED, COMMENTED])"`
Commits Commits `graphql:"commits(first: 1)"`
TimelineItems TimelineItems `graphql:"timelineItems(first: 1, itemTypes: [READY_FOR_REVIEW_EVENT])"`
} `graphql:"... on PullRequest"`
}
} `graphql:"search(query: $query, type: ISSUE, last: 50)"`
Expand Down
34 changes: 28 additions & 6 deletions cmd/ui.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,39 @@ func formatDuration(d time.Duration) string {
return duration
}

// getReadyForReviewOrPrCreatedAt returns when the pull request was
// marked ready for review, or its created date (if it was never in
// a draft state).
func getReadyForReviewOrPrCreatedAt(prCreated string, timelineItems TimelineItems) string {
if timelineItems.TotalCount == 0 {
return prCreated
}

return timelineItems.Nodes[0].ReadyForReviewEvent.CreatedAt
}

// getTimeToFirstReview returns the time to first review, in hours and
// minutes, for a given PR.
//
// firstReview = firstReviewAt - prCreatedAt
// timeToFirstReview = (readyForReviewAt || prCreatedAt) - firstReviewdAt
//
func (ui *UI) getTimeToFirstReview(prCreatedAtString string, reviews Reviews) string {
if len(reviews.Nodes) == 0 {
func (ui *UI) getTimeToFirstReview(author, prCreatedAt string, isDraft bool, timelineItems TimelineItems, reviews Reviews) string {
// The pull request is still in a draft state, because it has not
// yet been marked as ready for review.
if timelineItems.TotalCount == 0 && isDraft {
return DefaultEmptyCell
}

firstReviewedAt, _ := time.Parse(time.RFC3339, reviews.Nodes[0].CreatedAt)
prCreatedAt, _ := time.Parse(time.RFC3339, prCreatedAtString)
for _, review := range reviews.Nodes {
if review.Author.Login != author {
readyForReviewOrPrCreatedAt, _ := time.Parse(time.RFC3339, getReadyForReviewOrPrCreatedAt(prCreatedAt, timelineItems))
firstReviewedAt, _ := time.Parse(time.RFC3339, review.CreatedAt)

return formatDuration(ui.subtractTime(firstReviewedAt, readyForReviewOrPrCreatedAt))
}
}

return formatDuration(ui.subtractTime(firstReviewedAt, prCreatedAt.UTC()))
return DefaultEmptyCell
}

// getFeatureLeadTime returns the feature lead time, in hours and minutes,
Expand Down Expand Up @@ -175,7 +194,10 @@ func (ui *UI) PrintMetrics() string {
node.PullRequest.Deletions,
node.PullRequest.ChangedFiles,
ui.getTimeToFirstReview(
node.PullRequest.Author.Login,
node.PullRequest.CreatedAt,
node.PullRequest.IsDraft,
node.PullRequest.TimelineItems,
node.PullRequest.Reviews,
),
node.PullRequest.Comments.TotalCount,
Expand Down
76 changes: 69 additions & 7 deletions cmd/ui_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
"number": 5339,
"createdAt": "2022-03-21T15:11:09Z",
"changedFiles": 1,
"isDraft": false,
"mergedAt": "2022-03-21T16:22:05Z",
"participants": {
"totalCount": 3
Expand All @@ -46,21 +47,29 @@ const (
{
"author": {
"login": "Joker"
},
},
"createdAt": "2022-03-21T15:12:52Z",
"state": "APPROVED"
}
]
},
"commits": {
"totalCount": 4,
"totalCount": 1,
"nodes": [
{
"commit": {
"committedDate": "2022-03-21T15:09:52Z"
}
}
]
},
"timelineItems": {
"totalCount": 1,
"nodes": [
{
"createdAt": "2022-03-15T03:46:20Z"
}
]
}
}
]
Expand Down Expand Up @@ -125,7 +134,8 @@ func Test_SearchQuery_WithCSV(t *testing.T) {
Calendar: cal.NewBusinessCalendar(),
}

st.Assert(t, strings.Contains(ui.PrintMetrics(), "5339,4,6,3,1,2m,0,3,1h12m,--,1h9m"), true)
t.Log(ui.PrintMetrics())
st.Assert(t, strings.Contains(ui.PrintMetrics(), "5339,1,6,3,1,38h13m,0,3,1h12m,--,1h9m"), true)
}

func Test_subtractTime_WithinWorkday(t *testing.T) {
Expand Down Expand Up @@ -171,10 +181,39 @@ func Test_formatDuration_MoreThanMinute(t *testing.T) {
st.Assert(t, formatDuration(time.Minute*5), "5m")
}

func Test_getReadyForReviewOrPrCreatedAt_prCreatedAt(t *testing.T) {
st.Assert(t, getReadyForReviewOrPrCreatedAt("2022-03-21T15:11:09Z", TimelineItems{
TotalCount: 0,
}) == "2022-03-21T15:11:09Z", true)
}

func Test_getReadyForReviewOrPrCreatedAt_readyForReviewAt(t *testing.T) {
st.Assert(t, getReadyForReviewOrPrCreatedAt("2022-03-21T15:11:09Z", TimelineItems{
TotalCount: 1,
Nodes: TimelineItemNodes{
{ReadyForReviewEvent{CreatedAt: "2022-03-22T15:11:09Z"}},
}}) == "2022-03-22T15:11:09Z", true)
}

func Test_getTimeToFirstReview(t *testing.T) {
var timelineItems = TimelineItems{
TotalCount: 1,
Nodes: TimelineItemNodes{
{ReadyForReviewEvent{CreatedAt: "2022-03-21T15:11:09Z"}},
},
}
var reviews = Reviews{
Nodes: ReviewNodes{
{CreatedAt: "2022-03-22T15:11:09Z"},
{
Author: Author{Login: "Batman"},
CreatedAt: "2022-03-19T15:00:09Z",
State: "COMMENTED",
},
{
Author: Author{Login: "Joker"},
CreatedAt: "2022-03-20T15:11:09Z",
State: "APPROVED",
},
},
}

Expand All @@ -185,7 +224,7 @@ func Test_getTimeToFirstReview(t *testing.T) {
WorkdayEndFunc: WorkdayEnd,
},
}
st.Assert(t, uiWithWeekends.getTimeToFirstReview("2022-03-21T15:11:09Z", reviews), "24h0m")
st.Assert(t, uiWithWeekends.getTimeToFirstReview("Batman", "", false, timelineItems, reviews), "24h0m")

uiWithoutWeekends := &UI{
Calendar: &cal.BusinessCalendar{
Expand All @@ -194,16 +233,39 @@ func Test_getTimeToFirstReview(t *testing.T) {
WorkdayEndFunc: WorkdayEnd,
},
}
st.Assert(t, uiWithoutWeekends.getTimeToFirstReview("2022-03-21T15:11:09Z", reviews), "24h0m")
st.Assert(t, uiWithoutWeekends.getTimeToFirstReview("Batman", "", false, timelineItems, reviews), "15h11m")
}

func Test_getTimeToFirstReview_Draft(t *testing.T) {
var timelineItems = TimelineItems{
Nodes: TimelineItemNodes{},
}
var reviews = Reviews{
Nodes: ReviewNodes{
{
Author: Author{Login: "Joker"},
CreatedAt: "2022-03-22T15:11:09Z",
State: "COMMENTED",
},
},
}

ui := &UI{}
st.Assert(t, ui.getTimeToFirstReview("Batman", "", true, timelineItems, reviews), "--")
}

func Test_getTimeToFirstReview_NoReviews(t *testing.T) {
var timelineItems = TimelineItems{
Nodes: TimelineItemNodes{
{ReadyForReviewEvent{CreatedAt: "2022-03-21T15:11:09Z"}},
},
}
var reviews = Reviews{
Nodes: ReviewNodes{},
}

ui := &UI{}
st.Assert(t, ui.getTimeToFirstReview("2022-03-21T15:11:09Z", reviews), "--")
st.Assert(t, ui.getTimeToFirstReview("Batman", "", false, timelineItems, reviews), "--")
}

func Test_getFeatureLeadTime(t *testing.T) {
Expand Down

0 comments on commit f1ddce8

Please sign in to comment.