Skip to content

add analytics for how user leverages merge hint to merge#5550

Merged
outofambit merged 6 commits intodisplay-merge-status-in-compare-tabfrom
analytics-for-merge-hint
Sep 7, 2018
Merged

add analytics for how user leverages merge hint to merge#5550
outofambit merged 6 commits intodisplay-merge-status-in-compare-tabfrom
analytics-for-merge-hint

Conversation

@shiftkey
Copy link
Member

@shiftkey shiftkey commented Sep 5, 2018 •
edited by billygriffin
Loading

I'm not sure whether this entirely resolves (it doesn't completely) #5394 but I think this is two key metrics which will help us understand the impact of the new merge hint.

This PR checks at merge time what hint was shown to the user:

  • proceededAfterNoConflictsHintCount - the number of times a user merged with the "no conflicts" hint shown
  • proceededAfterConflictsWarningCount - the number of times a user merged with the "you have X conflicted files" metric

We already have some insight into how the user is initiating the merge flow (we aggregate these up into "total merges" which isn't quite accurate) but these two metrics show how many actually complete the flow and perform the merge to update their repository.

We don't report on the number of conflicted files shown by the dialog because we don't have a good story about sending those sorts of metrics, but we have that data available on mergeStatus.

This also requires changes to Central and desktop.github.com, but I'm opening this up for discussion to see whether it's on the right track.

cc @billygriffin @telliott27

@shiftkey shiftkey added the infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop label Sep 5, 2018
@shiftkey shiftkey added this to the 1.4.0 milestone Sep 5, 2018
@billygriffin
Copy link
Contributor

billygriffin commented Sep 5, 2018

@shiftkey This is a great start! A couple of thoughts/questions:

  1. What happens if a user clicks "merge" while the check for conflicts is pending? It seems like that wouldn't be captured by either of these metrics.
  2. @telliott27 and I talked about this, and we think the only thing missing that would resolve [Metrics] Merge conflict resolution #5394 is the eventual disposition of the merges that result in a conflict. In other words, how many of them are resolved "successfully" vs. abandoned? We were thinking a way to determine "successfully" might be if there was a commit in Desktop on that branch after a conflict was hit, but not exactly sure how to instrument that.

@shiftkey
Copy link
Member Author

shiftkey commented Sep 5, 2018

What happens if a user clicks "merge" while the check for conflicts is pending? It seems like that wouldn't be captured by either of these metrics.

Correct. I think those flows fall into one of these two buckets:

  • user went to merge immediately, without caring about the hint
  • user waited for the merge hint and got impatient waiting

we think the only thing missing that would resolve #5394 is the eventual disposition of the merges that result in a conflict. In other words, how many of them are resolved "successfully" vs. abandoned? We were thinking a way to determine "successfully" might be if there was a commit in Desktop on that branch after a conflict was hit, but not exactly sure how to instrument that.

Correct - there might be ways to identify these but I believe it's independent of this work (both the merge hint and this PR). We'd need to inspect the .git directory before committing to look for the presence of .git/MERGE_HEAD and .git/MERGE_MSG to indicate the current state of the repository.

@billygriffin
Copy link
Contributor

billygriffin commented Sep 5, 2018

@shiftkey 👍 Thoughts on adding the pending ones? Basically the goal is to capture the universe of merges actually initiated to have as the eventual denominator for "% of successful merges" type of metrics.

I'm fine with punting on the second portion, and your description of what's needed sounds great.

@shiftkey
Copy link
Member Author

shiftkey commented Sep 5, 2018

@billygriffin 👍 I'll add it in

@shiftkey
Copy link
Member Author

shiftkey commented Sep 5, 2018

I've settled on these three names:

  • mergedWithLoadingHintCount
  • mergedWithCleanMergeHintCount
  • mergedWithConflictWarningHintCount

If we're good to go with those, I'll open up tasks to track the other infrastructure changes we need.

@billygriffin
Copy link
Contributor

billygriffin commented Sep 5, 2018

All of those work for me @shiftkey! Thanks!

@shiftkey shiftkey mentioned this pull request Sep 5, 2018
Closed
8 tasks
iAmWillShepherd added a commit that referenced this pull request Sep 6, 2018
@iAmWillShepherd
Add new metrics
5fa1ff0 
Metrics added in #5550
@iAmWillShepherd iAmWillShepherd mentioned this pull request Sep 6, 2018
Merged
@iAmWillShepherd
Copy link
Contributor

iAmWillShepherd commented Sep 6, 2018

@billygriffin are you able to provide any justification for these new metrics over on #5556 ?

cc @desktop/analytics

@billygriffin
Copy link
Contributor

billygriffin commented Sep 6, 2018

@iAmWillShepherd Done, thanks!

@shiftkey shiftkey added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 7, 2018
outofambit
outofambit approved these changes Sep 7, 2018
View reviewed changes
Copy link
Contributor

@outofambit outofambit left a comment

Choose a reason for hiding this comment

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

this all sounds and looks good to me!

@outofambit outofambit merged commit d89bbb0 into display-merge-status-in-compare-tab Sep 7, 2018
@outofambit outofambit deleted the analytics-for-merge-hint branch September 7, 2018 17:29
@shiftkey shiftkey mentioned this pull request Sep 7, 2018
Closed
7 tasks
@outofambit outofambit deleted the analytics-for-merge-hint branch September 7, 2018 17:29
@shiftkey shiftkey mentioned this pull request Sep 7, 2018
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@niik niik Awaiting requested review from niik

2 more reviewers

@outofambit outofambit outofambit approved these changes

@iAmWillShepherd iAmWillShepherd iAmWillShepherd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@iAmWillShepherd iAmWillShepherd

Labels

infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

1.4.0

Development

Successfully merging this pull request may close these issues.

4 participants

@shiftkey @billygriffin @iAmWillShepherd @outofambit