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

Repairing xml comments #1846

Merged
merged 6 commits into from
Aug 13, 2018
Merged

Repairing xml comments #1846

merged 6 commits into from
Aug 13, 2018

Conversation

StanleyGoldman
Copy link
Contributor

@StanleyGoldman StanleyGoldman commented Aug 8, 2018 •
edited
Loading

Cleaning up xml comments

grokys
grokys reviewed Aug 8, 2018
View reviewed changes
src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs Outdated
/// <param name="pullRequestsService">The pull requests service.</param>
/// <param name="sessionManager">The pull request session manager.</param>
/// <param name="modelServiceFactory"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think simply "The model service factory" would suffice here.

src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs Outdated
/// <param name="usageTracker">The usage tracker.</param>
/// <param name="teamExplorerContext">The context for tracking repo changes</param>
/// <param name="files"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The view model which will display the changed files

src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs Outdated
/// <param name="usageTracker">The usage tracker.</param>
/// <param name="teamExplorerContext">The context for tracking repo changes</param>
/// <param name="files"></param>
/// <param name="syncSubmodulesCommand"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

A command that will be run when <see cref="SyncSubmodules"/> is executed

src/GitHub.Exports.Reactive/Services/IPullRequestService.cs Outdated
@@ -84,6 +85,7 @@ public interface IPullRequestService
/// Sync submodules on the current branch.
/// </summary>
/// <param name="repository">The repository.</param>
/// <param name="progress"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

A method that will be called with progress messages

src/GitHub.Exports/Models/ILocalRepositoryModel.cs Outdated
@@ -29,6 +29,7 @@ public interface ILocalRepositoryModel : IRepositoryModel, INotifyPropertyChange
/// Generates a http(s) url to the repository in the remote server, optionally
/// pointing to a specific file and specific line range in it.
/// </summary>
/// <param name="linkType"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The type of link to create?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The type of repository link to create

src/GitHub.InlineReviews/Commands/InlineCommentNavigationCommand.cs Outdated
@@ -217,7 +218,10 @@ protected IReadOnlyList<ITagInfo> GetTags(IEnumerable<ITextView> textViews)
/// <summary>
/// Shows the inline comments for the specified tag in a peek view.
/// </summary>
/// <param name="textView"></param>
/// <param name="tag"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The inline comment tag

src/GitHub.InlineReviews/Commands/InlineCommentNavigationCommand.cs Outdated
/// <param name="tag"></param>
/// <param name="parameter"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this one is difficult

The navigation parameter detailing a search from the specified tag

Or something?!

src/GitHub.InlineReviews/Commands/InlineCommentNavigationCommand.cs Outdated
/// <param name="tag"></param>
/// <param name="parameter"></param>
/// <param name="allTextViews"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The additional text views when showing a comment in a diff view or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The full list of text views

src/GitHub.InlineReviews/Models/InlineCommentThreadModel.cs Outdated
/// <param name="diffMatch">
/// The last five lines of the thread's diff hunk, in reverse order.
/// </param>
/// <param name="comments"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The comments in the thread

src/GitHub.InlineReviews/Services/IInlineCommentPeekService.cs Outdated
@@ -15,6 +15,7 @@ public interface IInlineCommentPeekService
/// Gets the line number for a peek session tracking point.
/// </summary>
/// <param name="session">The peek session.</param>
/// <param name="point"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The peek session tracking point

@StanleyGoldman StanleyGoldman changed the title [WIP] Repairing xml comments Repairing xml comments Aug 9, 2018
StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

asdf

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

Going back in time

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

Great Scott Marty! To the DeLorean!

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

I'm gonna be mayor of this town

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

I'm gonna be mayor of this town

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

I'm gonna be mayor of this town

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

I'm gonna be mayor of this town

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

I'm gonna be mayor of this town

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

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

Someday I'm gonna be mayor of this town

StanleyGoldman
StanleyGoldman commented Aug 9, 2018
View reviewed changes
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You want that when?

jcansdale
jcansdale reviewed Aug 9, 2018
View reviewed changes
src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs
@@ -23,7 +23,7 @@ public interface IRepositoryCloneService
/// <param name="repositoryPath">The directory that will contain the repository directory.</param>
/// <param name="progress">
/// An object through which to report progress. This must be of type
/// <see cref="System.IProgress{Microsoft.VisualStudio.Shell.ServiceProgressData}"/>, but
/// System.IProgress&lt;Microsoft.VisualStudio.Shell.ServiceProgressData&gt;, but
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened here?

Copy link
Contributor Author

@StanleyGoldman StanleyGoldman Aug 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

It was an incorrect <see> tag. The "cref" value could provide a link to documentation for System.IProgress{T} or Microsoft.VisualStudio.Shell.ServiceProgressData, but there is no link to documentation for System.IProgress{Microsoft.VisualStudio.Shell.ServiceProgressData}.

grokys
grokys reviewed Aug 10, 2018
View reviewed changes
src/GitHub.App/Extensions/AkavacheExtensions.cs Outdated
@@ -252,11 +252,12 @@ public static class AkavacheExtensions
/// <typeparam name="T"></typeparam>
/// <param name="blobCache">The cache to retrieve the object from.</param>
/// <param name="key">The key to look up the cache value with.</param>
/// <param name="item">The item to add to the database</param>
/// <param name="fetchFunc"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The fetch function - this is what the other params like like in the same file are documented with.

src/GitHub.App/Services/PullRequestEditorService.cs
@@ -336,6 +336,7 @@ public IVsTextView FindActiveView()
/// <param name="fromLines">The document we're navigating from.</param>
/// <param name="toLines">The document we're navigating to.</param>
/// <param name="line">The 0-based line we're navigating from.</param>
/// <param name="matchLinesAbove"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The number of lines to match.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jcansdale shouldn't the default for this parameter be MatchLinesAboveTarget instead of 0?

src/GitHub.Exports/Services/IGitHubContextService.cs
/// <returns>A repository URL</returns>
GitHubContext FindContextFromWindowTitle(string windowTitle);

/// <summary>
/// Find a context from a browser window title.
/// </summary>
/// <param name="windowTitle">A browser window title.</param>
/// <param name="context"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The context? Not very descriptive. Do we need more info than this? @jcansdale?

src/GitHub.InlineReviews/ViewModels/PullRequestReviewCommentViewModel.cs Outdated
@@ -88,6 +88,7 @@ public class PullRequestReviewCommentViewModel : CommentViewModel, IPullRequestR
/// <param name="commentService">Comment Service</param>
/// <param name="thread">The thread that the comment is a part of.</param>
/// <param name="currentUser">The current user.</param>
/// <param name="review"></param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The associated pull request review.

@grokys grokys merged commit aeb7f79 into master Aug 13, 2018
@grokys grokys deleted the xml-doc-comments branch August 13, 2018 13:58
@meaghanlewis meaghanlewis added this to the 2.5.5 milestone Aug 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jcansdale jcansdale jcansdale left review comments

@grokys grokys grokys approved these changes

Assignees
No one assigned
Labels
Ready for Review
Projects
None yet
Milestone
2.5.5
Development

Successfully merging this pull request may close these issues.
4 participants
@StanleyGoldman @grokys @jcansdale @meaghanlewis