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

Conversation

@grokys
Copy link
Contributor

@grokys grokys commented Aug 3, 2017

This PR adds the following metrics:

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

Depends on #1036

@grokys grokys changed the title WIP: Add metrics for inline comments Add metrics for inline comments Aug 7, 2017
@grokys grokys requested review from jcansdale and shana August 7, 2017 16:37
grokys added 6 commits August 9, 2017 12:49
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 jcansdale force-pushed the feature/inline-comments-metrics branch from 41b204a to 8927658 Compare August 9, 2017 12:49
return null;
}

/// <inheritdoc/>
Copy link
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
Contributor Author

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
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?

Copy link
Collaborator

@jcansdale jcansdale left a comment

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?

Copy link
Collaborator

@jcansdale jcansdale left a comment

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 master Aug 9, 2017
@jcansdale jcansdale deleted the feature/inline-comments-metrics branch August 9, 2017 14:09
Sign up for free to 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.

3 participants