From a67449a82220a7a1c30d4db2bc081b00c2a8df24 Mon Sep 17 00:00:00 2001 From: William Shepherd Date: Thu, 7 Feb 2019 10:54:30 -0600 Subject: [PATCH 1/9] Add new metrics to track in db Co-Authored-By: Billy Griffin --- app/src/lib/stats/stats-database.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/src/lib/stats/stats-database.ts b/app/src/lib/stats/stats-database.ts index 8d974b0eb4..6c1eabad46 100644 --- a/app/src/lib/stats/stats-database.ts +++ b/app/src/lib/stats/stats-database.ts @@ -61,6 +61,7 @@ export interface IDailyMeasures { /** The numbers of times a repo with indicators is clicked on repo list view */ readonly repoWithIndicatorClicked: number + /** The numbers of times a repo without indicators is clicked on repo list view */ readonly repoWithoutIndicatorClicked: number @@ -141,6 +142,12 @@ export interface IDailyMeasures { /** The number of times the user committed a conflicted merge outside the merge conflicts dialog */ readonly unguidedConflictedMergeCompletionCount: number + + /** The number of times the user is taken to the create pull request page on dotcom from the branch menu */ + readonly createPullRequestFromMenuCount: number + + /** The number of times the user is taken to the create pull request page on dotcom from the suggested next steps flow */ + readonly createPullRequestFromSuggestedNextStepCount: number } export class StatsDatabase extends Dexie { From 084606478197f7d7793d6325b19610e28183df60 Mon Sep 17 00:00:00 2001 From: William Shepherd Date: Thu, 7 Feb 2019 10:54:51 -0600 Subject: [PATCH 2/9] Set initial values Co-Authored-By: Billy Griffin --- app/src/lib/stats/stats-store.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/lib/stats/stats-store.ts b/app/src/lib/stats/stats-store.ts index 4ca96809b2..3d430df65a 100644 --- a/app/src/lib/stats/stats-store.ts +++ b/app/src/lib/stats/stats-store.ts @@ -80,6 +80,8 @@ const DefaultDailyMeasures: IDailyMeasures = { mergeConflictsDialogReopenedCount: 0, guidedConflictedMergeCompletionCount: 0, unguidedConflictedMergeCompletionCount: 0, + createPullRequestFromMenuCount: 0, + createPullRequestFromSuggestedNextStepCount: 0, } interface IOnboardingStats { From b1817c32151294300f4f1676edf35970901ac5a7 Mon Sep 17 00:00:00 2001 From: William Shepherd Date: Thu, 7 Feb 2019 10:55:08 -0600 Subject: [PATCH 3/9] Add way to increment metrics Co-Authored-By: Billy Griffin --- app/src/lib/stats/stats-store.ts | 21 +++++++++++++++++++++ app/src/ui/dispatcher/dispatcher.ts | 14 ++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/src/lib/stats/stats-store.ts b/app/src/lib/stats/stats-store.ts index 3d430df65a..e3480d9446 100644 --- a/app/src/lib/stats/stats-store.ts +++ b/app/src/lib/stats/stats-store.ts @@ -821,6 +821,27 @@ export class StatsStore implements IStatsStore { })) } + /** + * Increments the `createPullRequestFromMenuCount` metric + */ + public async recordCreatePullRequestFromMenuCount(): Promise { + return this.updateDailyMeasures(m => ({ + createPullRequestFromMenuCount: m.createPullRequestFromMenuCount + 1, + })) + } + + /** + * Increments the `createPullRequestFromSuggestedNextStepCount` metric + */ + public async recordCreatePullRequestFromSuggestedNextStepCount(): Promise< + void + > { + return this.updateDailyMeasures(m => ({ + createPullRequestFromSuggestedNextStepCount: + m.createPullRequestFromSuggestedNextStepCount + 1, + })) + } + public recordWelcomeWizardInitiated() { setNumber(WelcomeWizardInitiatedAtKey, Date.now()) localStorage.removeItem(WelcomeWizardCompletedAtKey) diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 106a4b7c4b..e877ddce8e 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -1421,6 +1421,20 @@ export class Dispatcher { return this.statsStore.recordDivergingBranchBannerInitatedMerge() } + /** + * Increments the `CreatePullRequestFromMenuCount` metric + */ + public recordCreatePullRequestFromMenu() { + return this.statsStore.recordCreatePullRequestFromMenuCount() + } + + /** + * Increments the `CreatePullRequestFromSuggestedNextStepCount` metric + */ + public recordCreatePullRequestFromSuggestedNextStep() { + return this.statsStore.recordCreatePullRequestFromSuggestedNextStepCount() + } + public recordWelcomeWizardInitiated() { return this.statsStore.recordWelcomeWizardInitiated() } From 9caba2d13e57640188ca885499fe95ed14611d80 Mon Sep 17 00:00:00 2001 From: William Shepherd Date: Thu, 7 Feb 2019 10:55:18 -0600 Subject: [PATCH 4/9] Increment when menu is clicked Co-Authored-By: Billy Griffin --- app/src/ui/app.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index 1ac78b5a9d..e7faaa0aaf 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -1832,6 +1832,7 @@ export class App extends React.Component { if (currentPullRequest == null) { dispatcher.createPullRequest(state.repository) + dispatcher.recordCreatePullRequestFromMenu() } else { dispatcher.showPullRequest(state.repository) } From dddefd371a8ead5b5dcdf4ee9634d6befc2daed1 Mon Sep 17 00:00:00 2001 From: William Shepherd Date: Thu, 7 Feb 2019 10:55:43 -0600 Subject: [PATCH 5/9] Increment when creating pr via next steps flow Co-Authored-By: Billy Griffin --- app/src/ui/branches/branches-container.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/ui/branches/branches-container.tsx b/app/src/ui/branches/branches-container.tsx index a4c47092ac..31cd6e9e5f 100644 --- a/app/src/ui/branches/branches-container.tsx +++ b/app/src/ui/branches/branches-container.tsx @@ -264,6 +264,7 @@ export class BranchesContainer extends React.Component< private onCreatePullRequest = () => { this.props.dispatcher.closeFoldout(FoldoutType.Branch) this.props.dispatcher.createPullRequest(this.props.repository) + this.props.dispatcher.recordCreatePullRequestFromSuggestedNextStep() } private onPullRequestClicked = (pullRequest: PullRequest) => { From fc605d1e3a78f523e290aa853d4967919b4963d2 Mon Sep 17 00:00:00 2001 From: William Shepherd Date: Thu, 7 Feb 2019 11:10:00 -0600 Subject: [PATCH 6/9] No need to differentiate Co-Authored-By: Billy Griffin --- app/src/lib/stats/stats-database.ts | 7 ++----- app/src/lib/stats/stats-store.ts | 21 ++++----------------- app/src/ui/branches/branches-container.tsx | 1 - app/src/ui/dispatcher/dispatcher.ts | 9 +-------- 4 files changed, 7 insertions(+), 31 deletions(-) diff --git a/app/src/lib/stats/stats-database.ts b/app/src/lib/stats/stats-database.ts index 6c1eabad46..a831a6f666 100644 --- a/app/src/lib/stats/stats-database.ts +++ b/app/src/lib/stats/stats-database.ts @@ -143,11 +143,8 @@ export interface IDailyMeasures { /** The number of times the user committed a conflicted merge outside the merge conflicts dialog */ readonly unguidedConflictedMergeCompletionCount: number - /** The number of times the user is taken to the create pull request page on dotcom from the branch menu */ - readonly createPullRequestFromMenuCount: number - - /** The number of times the user is taken to the create pull request page on dotcom from the suggested next steps flow */ - readonly createPullRequestFromSuggestedNextStepCount: number + /** The number of times the user is taken to the create pull request page on dotcom */ + readonly createPullRequestCount: number } export class StatsDatabase extends Dexie { diff --git a/app/src/lib/stats/stats-store.ts b/app/src/lib/stats/stats-store.ts index e3480d9446..3bb0b4ceb2 100644 --- a/app/src/lib/stats/stats-store.ts +++ b/app/src/lib/stats/stats-store.ts @@ -80,8 +80,7 @@ const DefaultDailyMeasures: IDailyMeasures = { mergeConflictsDialogReopenedCount: 0, guidedConflictedMergeCompletionCount: 0, unguidedConflictedMergeCompletionCount: 0, - createPullRequestFromMenuCount: 0, - createPullRequestFromSuggestedNextStepCount: 0, + createPullRequestCount: 0, } interface IOnboardingStats { @@ -822,23 +821,11 @@ export class StatsStore implements IStatsStore { } /** - * Increments the `createPullRequestFromMenuCount` metric + * Increments the `createPullRequestCount` metric */ - public async recordCreatePullRequestFromMenuCount(): Promise { + public async recordCreatePullRequest(): Promise { return this.updateDailyMeasures(m => ({ - createPullRequestFromMenuCount: m.createPullRequestFromMenuCount + 1, - })) - } - - /** - * Increments the `createPullRequestFromSuggestedNextStepCount` metric - */ - public async recordCreatePullRequestFromSuggestedNextStepCount(): Promise< - void - > { - return this.updateDailyMeasures(m => ({ - createPullRequestFromSuggestedNextStepCount: - m.createPullRequestFromSuggestedNextStepCount + 1, + createPullRequestCount: m.createPullRequestCount + 1, })) } diff --git a/app/src/ui/branches/branches-container.tsx b/app/src/ui/branches/branches-container.tsx index 31cd6e9e5f..a4c47092ac 100644 --- a/app/src/ui/branches/branches-container.tsx +++ b/app/src/ui/branches/branches-container.tsx @@ -264,7 +264,6 @@ export class BranchesContainer extends React.Component< private onCreatePullRequest = () => { this.props.dispatcher.closeFoldout(FoldoutType.Branch) this.props.dispatcher.createPullRequest(this.props.repository) - this.props.dispatcher.recordCreatePullRequestFromSuggestedNextStep() } private onPullRequestClicked = (pullRequest: PullRequest) => { diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index e877ddce8e..584d248b51 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -1425,14 +1425,7 @@ export class Dispatcher { * Increments the `CreatePullRequestFromMenuCount` metric */ public recordCreatePullRequestFromMenu() { - return this.statsStore.recordCreatePullRequestFromMenuCount() - } - - /** - * Increments the `CreatePullRequestFromSuggestedNextStepCount` metric - */ - public recordCreatePullRequestFromSuggestedNextStep() { - return this.statsStore.recordCreatePullRequestFromSuggestedNextStepCount() + return this.statsStore.recordCreatePullRequest() } public recordWelcomeWizardInitiated() { From f168a8dbb02e3e6550517a363ed56b3db35e1ff0 Mon Sep 17 00:00:00 2001 From: William Shepherd Date: Thu, 7 Feb 2019 11:16:38 -0600 Subject: [PATCH 7/9] Add documentation Co-Authored-By: Billy Griffin --- app/src/ui/app.tsx | 2 +- app/src/ui/dispatcher/dispatcher.ts | 4 ++-- docs/process/metrics.md | 11 ++++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index e7faaa0aaf..76f64a89e6 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -1832,7 +1832,7 @@ export class App extends React.Component { if (currentPullRequest == null) { dispatcher.createPullRequest(state.repository) - dispatcher.recordCreatePullRequestFromMenu() + dispatcher.recordCreatePullRequest() } else { dispatcher.showPullRequest(state.repository) } diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 584d248b51..a82b838e79 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -1422,9 +1422,9 @@ export class Dispatcher { } /** - * Increments the `CreatePullRequestFromMenuCount` metric + * Increments the `CreatePullRequestCount` metric */ - public recordCreatePullRequestFromMenu() { + public recordCreatePullRequest() { return this.statsStore.recordCreatePullRequest() } diff --git a/docs/process/metrics.md b/docs/process/metrics.md index c261785c8b..7d13023ef6 100644 --- a/docs/process/metrics.md +++ b/docs/process/metrics.md @@ -30,9 +30,11 @@ These are general metrics about feature usage and specific feature behaviors. Th | Metric | Description | Justification | |:--|:--|:--| | `active*` | Flag indicating whether the app has been interacted with during the current reporting window. | To identify users who are actively using Desktop versus those who have it open but never interact with it. | +| `anyConflictsLeftOnMergeConflictsDialogDismissalCount` | The number of times there were any merge conflicts present when the Merge Conflicts Dialog is dismissed. | To understand whether people dismiss the dialog after resolving conflicts for one last check or just want to back out of the guided flow entirely. | | `branchComparisons` | The number of times a branch is compared to an arbitrary branch. | To understand usage patterns around the compare branches feature. | | `coAuthoredCommits` | The number of commits created with one or more co-authors. | To understand usage patterns of commits made in Desktop. | | `commits` | The number of commits made. | To understand usage patterns of commits made in Desktop. | +| `createPullRequestCount` | The number of times the user is taken to the create pull request page on GitHub.com. | To understand how people are creating pull requests via Desktop. | | `defaultBranchComparisons` | The number of times a branch is compared to the default branch. | To understand usage patterns around the compare branches feature. | | `divergingBranchBannerDismissal` | The number of times the user dismisses the diverged branch notification. | To understand usage patterns around the notification of diverging from the default branch feature. | | `divergingBranchBannerDisplayed` | The number of times the diverged branch notification is displayed. | To understand usage patterns around the notification of diverging from the default branch feature. | @@ -41,11 +43,14 @@ These are general metrics about feature usage and specific feature behaviors. Th | `divergingBranchBannerInitiatedCompare` | The number of times the user compares from the diverged branch notification compare CTA button. | To understand usage patterns around the notification of diverging from the default branch feature. | | `dotcomCommits` | The number of time the user made a commit to a repo hosted on Github.com. | To understand the total percentage of commits made to GitHub repos compared to GitHub Enterprise and other remotes to help prioritize our work and focus areas | | `enterpriseCommits` | The number of times the user made a commit to a repo hosted on a GitHub Enterprise instance. | To understand the total percentage of commits made to GitHub Enterprise repos to help prioritize our work associated with enterprise use of GitHub Desktop compared to GitHub | +| `guidedConflictedMergeCompletionCount` | The number of times a conflicted merge is completed from the Merge Conflicts Dialog. | To understand how many times people prefer to finish the merge in the guided flow after resolving conflicts. | | `loadTime` | The time (in milliseconds) it takes from when loading begins to loading end. | To make sure new versions of Desktop are not regressing on performance. | | `mainReadyTime` | The time (in milliseconds) it takes from when our main process code is first loaded until the app `ready` event is emitted. | To make sure new versions of Desktop are not regressing on performance. | | `mergeAbortedAfterConflictsCount` | The number of times the user aborts a merge after a merge conflict. | To understand the frequency of merges that are never completed after attempting to merge and hitting a merge conflict | | `mergeConflictFromExplicitMergeCount` | The number of times a `git merge` initiated by Desktop resulted in a merge conflict for the user. | To understand how often people encounter a merge conflict in Desktop. | | `mergeConflictFromPullCount` | The number of times a `git pull` initiated by Desktop resulted in a merge conflict for the user. | To understand how often people encounter a merge conflict in Desktop. | +| `mergeConflictsDialogDismissalCount` | The number of times the Merge Conflicts Dialog is dismissed. | To understand how frequently people prefer a different merge conflicts flow than the guided one. | +| `mergeConflictsDialogReopenedCount` | The number of times the Merge Conflicts Dialog is reopened from the Merge Conflicts Banner. | To understand whether people find value in both the guided and unguided merge conflicts flow. | | `mergedWithCleanMergeHintCount` | The number of times the user has merged after seeing the 'no conflicts' merge hint. | To understand how many "clean" merges there are | | `mergedWithConflictWarningHintCount` | The number of times the user has merged after seeing the 'you have XX conflicted files' warning. | To understand how frequently people are merging even though they know there will be conflicts | | `mergedWithLoadingHintCount` | The number of times the user merged before seeing the result of the merge hint. | To understand how many people are merging before learning whether there will be conflicts or not | @@ -59,9 +64,5 @@ These are general metrics about feature usage and specific feature behaviors. Th | `repoWithIndicatorClicked` | The numbers of times a repo with indicators is clicked on repo list view. | To understand usage patterns around the repository indicators feature. | | `repoWithoutIndicatorClicked` | The numbers of times a repo without indicators is clicked on repo list view. | To understand usage patterns around the repository indicators feature. | | `unattributedCommits` | The number of commits that will go unattributed to GitHub users. | To understand how frequently commits in GitHub Desktop are unattributed and how highly we should prioritize design for those instances | -| `updateFromDefaultBranchMenuCount` | The number of times the `Branch -> Update From Default Branch` menu item is used. | To understand usage patterns around the compare branches feature. | -| `mergeConflictsDialogDismissalCount` | The number of times the Merge Conflicts Dialog is dismissed. | To understand how frequently people prefer a different merge conflicts flow than the guided one. | -| `anyConflictsLeftOnMergeConflictsDialogDismissalCount` | The number of times there were any merge conflicts present when the Merge Conflicts Dialog is dismissed. | To understand whether people dismiss the dialog after resolving conflicts for one last check or just want to back out of the guided flow entirely. | -| `mergeConflictsDialogReopenedCount` | The number of times the Merge Conflicts Dialog is reopened from the Merge Conflicts Banner. | To understand whether people find value in both the guided and unguided merge conflicts flow. | -| `guidedConflictedMergeCompletionCount` | The number of times a conflicted merge is completed from the Merge Conflicts Dialog. | To understand how many times people prefer to finish the merge in the guided flow after resolving conflicts. | | `unguidedConflictedMergeCompletionCount` | The number of times a conflicted merge is completed from the diff/changes view. | To understand how many times people prefer to finish the merge in the changes/diff view after resolving conflicts. | +| `updateFromDefaultBranchMenuCount` | The number of times the `Branch -> Update From Default Branch` menu item is used. | To understand usage patterns around the compare branches feature. | From 98068787bf070f1cd50bd1b961d0039980fbaf7d Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Thu, 7 Feb 2019 13:11:11 -0700 Subject: [PATCH 8/9] Fix capitalization in comment Co-Authored-By: billygriffin <5091167+billygriffin@users.noreply.github.com> --- app/src/ui/dispatcher/dispatcher.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index a82b838e79..f3ebb87e76 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -1422,7 +1422,7 @@ export class Dispatcher { } /** - * Increments the `CreatePullRequestCount` metric + * Increments the `createPullRequestCount` metric */ public recordCreatePullRequest() { return this.statsStore.recordCreatePullRequest() From 9f39e4f362c78cb35031388d5ebee7481e6aec8d Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Thu, 7 Feb 2019 16:13:45 -0400 Subject: [PATCH 9/9] cherry-pick config fix to get Azure Pipelines building --- azure-pipelines.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 256a0f074b..d778ad276a 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -25,7 +25,7 @@ jobs: displayName: 'Publish Test Results' inputs: testResultsFiles: '**junit*.xml' - testRunTitle: TestRun ${{ parameters.name }} $(node_version) + testRunTitle: $(Agent.OS) Test Results - task: PublishCodeCoverageResults@1 displayName: 'Publish code coverage results' inputs: @@ -60,7 +60,7 @@ jobs: displayName: 'Publish Test Results' inputs: testResultsFiles: '**junit*.xml' - testRunTitle: TestRun ${{ parameters.name }} $(node_version) + testRunTitle: $(Agent.OS) Test Results - task: PublishCodeCoverageResults@1 displayName: 'Publish code coverage results' inputs: @@ -87,7 +87,7 @@ jobs: displayName: 'Publish Test Results' inputs: testResultsFiles: '**junit*.xml' - testRunTitle: TestRun ${{ parameters.name }} $(node_version) + testRunTitle: $(Agent.OS) Test Results - task: PublishCodeCoverageResults@1 displayName: 'Publish code coverage results' inputs: @@ -113,3 +113,6 @@ jobs: - script: | yarn lint name: Lint + - script: | + yarn check-modified + displayName: 'Check for modified files'