Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename Comment to ReviewComment #1520

Merged
merged 4 commits into from
Dec 31, 2016
Merged

Conversation

bmeverett
Copy link
Contributor

@bmeverett bmeverett commented Dec 21, 2016

Fixes #1511

Change references of Comment to ReviewComment based on the API change

  • Rename Comment to ReviewComment in the PullRequestClient and interface as well as all tests related to it.
  • Replace the Observable interfaces and classes

@bmeverett
Copy link
Contributor Author

I changed this in the PullRequestClient as well as the ObservablePullRequestClient Should we change it anywhere else at this time?

Copy link
Contributor

@ryangribble ryangribble left a comment

Choose a reason for hiding this comment

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

Great work so far! I've highlighted a couple of formatting/doc issues and also pointed out that the obsoleted Comment endpoints won't currently work since they will be null ! 😁

IObservablePullRequestReviewCommentsClient Comment { get; }
IObservablePullRequestReviewCommentsClient ReviewComment { get; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Separate the old member and the new member with a line of whitespace, and dont forget to add XmlDoc for the new member

eg:

/// <summary>
/// Client for managing comments.
/// </summary>
[Obsolete("Please use IObservablePullRequestsClient.ReviewComment. This will be removed in a future version")]
IObservablePullRequestReviewCommentsClient Comment { get; }		          

/// <summary>
/// Client for managing review comments.
/// </summary>
IObservablePullRequestReviewCommentsClient ReviewComment { get; }

@@ -18,15 +18,17 @@ public class ObservablePullRequestsClient : IObservablePullRequestsClient
/// <summary>
/// Client for managing comments.
/// </summary>
public IObservablePullRequestReviewCommentsClient Comment { get; private set; }
[Obsolete("Please use ObservablePullRequestsClient.ReviewComment. This will be removed in a future version")]
public IObservablePullRequestReviewCommentsClient Comment { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need the Comment endpoint to function so let's make it a getter only, and have it return the new client

eg

public IObservablePullRequestReviewCommentsClient Comment { get { return this.ReviewComment; } }

public IObservablePullRequestReviewCommentsClient Comment { get; private set; }
[Obsolete("Please use ObservablePullRequestsClient.ReviewComment. This will be removed in a future version")]
public IObservablePullRequestReviewCommentsClient Comment { get; set; }
public IObservablePullRequestReviewCommentsClient ReviewComment { get; private set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments about whitespace and XmlDoc here

IPullRequestReviewCommentsClient Comment { get; }
/// <summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

fix formatting (whitespace) here for consistency

IPullRequestReviewCommentsClient Comment { get; }
/// <summary>
/// Client for managing comments.
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this comment should say "Client for managing review comments" now?

public IPullRequestReviewCommentsClient Comment { get; private set; }

[Obsolete("Please use PullRequestsClient.ReviewComment instead. This method will be removed in a future version")]
public IPullRequestReviewCommentsClient Comment { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

same comments around getter only, whitespace, and xmldoc wording change

have Comment return new ReviewComment
@bmeverett
Copy link
Contributor Author

@ryangribble I have made the requested changes.

@ryangribble
Copy link
Contributor

Great work... Many thanks for your contribution, this is good to go!

@ryangribble ryangribble merged commit 155073b into octokit:master Dec 31, 2016
@ryangribble ryangribble changed the title [WIP] Rename Comment to ReviewComment Change location of PullRequestReviewCommentsClient from PullRequest.Comment to PullRequest.ReviewComment Jan 15, 2017
@ryangribble ryangribble changed the title Change location of PullRequestReviewCommentsClient from PullRequest.Comment to PullRequest.ReviewComment Rename Comment to ReviewComment Jan 15, 2017
@ryangribble
Copy link
Contributor

ryangribble commented Jan 15, 2017

release_notes: Rename PullRequest.Comment to PullRequest.ReviewComment for better accuracy

@nickfloyd nickfloyd added Type: Feature New feature or request and removed category: feature labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants