Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Add metrics for inline comments - #1107

Merged
jcansdale merged 6 commits into
masterfrom
feature/inline-comments-metrics
Aug 9, 2017
Merged

Add metrics for inline comments#1107
jcansdale merged 6 commits into
masterfrom
feature/inline-comments-metrics

Conversation

@grokys

@grokysgrokys commented Aug 3, 2017

Copy link
Copy Markdown
Contributor

This PR adds the following metrics:

  • NumberOfPRDetailsViewChanges
  • NumberOfPRDetailsViewFile
  • NumberOfPRDetailsCompareWithSolution
  • NumberOfPRDetailsOpenFileInSolution
  • NumberOfPRReviewDiffViewInlineCommentOpen
  • NumberOfPRReviewDiffViewInlineCommentPost

Depends on #1036

@grokysgrokys changed the title WIP: Add metrics for inline commentsAdd metrics for inline commentsAug 7, 2017
@grokys
grokys requested review from jcansdale and shanaAugust 7, 2017 16:37
Previously `InlineCommentThreadViewModel` and `NewInlineCommentThreadViewModel` posted comments directly to `IApiClient` and then notified the `PullRequestSession` of the added comment. Simplified this by adding the functionality to post review comments to `PullRequestSession`.
@jcansdale
jcansdaleforce-pushed the feature/inline-comments-metrics branch from 41b204a to 8927658CompareAugust 9, 2017 12:49
return null;
}

/// <inheritdoc/>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should these methods be added as part of this PR or have they snuck in from somewhere else?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

They're part of the PR - I had to move things around a bit in order to be able to get hold of an IUsageTracker at the right place.

}

/// <inheritdoc/>
public async Task<IPullRequestReviewCommentModel> PostReviewComment(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

More new methods. This this PR depend on some other PR that hasn't been merged yet?

@jcansdalejcansdale left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There seems to be some non-metrics related stuff in this PR. Has this snuck in from some other PR?

@jcansdalejcansdale left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's do this!

@jcansdale
jcansdale merged commit c502cf8 into masterAug 9, 2017
@jcansdale
jcansdale deleted the feature/inline-comments-metrics branch August 9, 2017 14:09
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@grokys@jcansdale