Skip to content

Support Reactions of Comments #266

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

Open
3 tasks
TylerLeonhardt opened this issue Jul 16, 2020 · 5 comments
Open
3 tasks

Support Reactions of Comments #266

TylerLeonhardt opened this issue Jul 16, 2020 · 5 comments
Labels
api-reactions Work to complete the API's defined here: https://developer.github.com/v3/reactions discussion We are looking for additional community feedback on this topic before proceeding further. enhancement An issue or pull request introducing new functionality to the project. triage needed An issue that needs to be reviewed by a member of the team.

Comments

@TylerLeonhardt
Copy link
Member

Feature Idea Summary

Now that #193 is in, this will track reacting to

  • Issue Comments
  • PR Comments
  • Commit Comments

Feature Idea Additional Details

In #193, I proposed:

For Get-GitHubComment I've can add PullRequestNumber and IssueNumber

Get-GitHubReaction will have a parameter set just for Issue (PullRequestNumber will then be alias so that pipeline works) with an optional Comment parameter. When we support Pull Request Comments we can split up into 2 parameter sets:

PullRequestComment
PullRequestReviewComment

where we add a ReviewComment param

This would also align with Get-GitHubComment which can get PR Comments today.

Requested Assignment

  • If possible, I would like to implement this.
@TylerLeonhardt TylerLeonhardt added enhancement An issue or pull request introducing new functionality to the project. triage needed An issue that needs to be reviewed by a member of the team. labels Jul 16, 2020
@HowardWolosky HowardWolosky added discussion We are looking for additional community feedback on this topic before proceeding further. api-reactions Work to complete the API's defined here: https://developer.github.com/v3/reactions labels Jul 16, 2020
@HowardWolosky
Copy link
Contributor

Some initial thoughts:

#242 renamed Get-GitHubComment -> Get-GetHubIssueComment because it's really just focused on Issue comments. It was aliased back to Get-GitHubComment to avoid a breaking change, but it's really an Issue-focused function.

I think it's going to be difficult to create a single function that can handle all of the different comment types. For instance, getting a specific pull request comment does not require having the pull request number available. Neither do you need the issue number to get a specific issue comment . We could in theory author the function to require the PullRequestNumber/IssueNumber, which would make the signature much more straight-forward, but trying to enable the minimum required parameters of the API means that we can't cover both types in the same function. It's a similar situation with reactions to those other comment types. We don't technically need that information to successfully use those API's, so it seems a bit artificial to require them of the user.

I also don't think that adding an additional string parameter to capture the type of comment is a good alternative either. E.g.

[ValidateSet('IssueComment', 'PullRequestComment', 'PullRequestReviewComment']
[string] $CommentType

Feel free to prove me wrong though or provide a compelling argument.

Right now, you can see that in #172 I added Get-GitHubGistComment (which is yet another comment type, but one that doesn't support reactions), and in my ongoing pullRequests work it adds Get-GitHubPullRequestReviewComment.

Maybe we do move forward with your current designed plan and have Get-GitHubComment/Get-GitHubReaction support all comment types, but additionally require the unnecessary parameters in order to qualify what the CommentId is referencing, but then also add type-specific functions (e.g. Get-GitHubIssueComment, Get-GitHubPullRequestComment, Get-GitHubPullRequestReviewComment, etc...) that don't require that additional data given the context is already there in the name.

Thoughts?

@TylerLeonhardt
Copy link
Member Author

Do you think there's a scenario when you want only Pull Request review comments and not Pull Request normal comments?

I'm thinking about:

Get-GitHubComment -PullRequest 1234

What would a user expect that to do?

Personally - I'm thinking it'd grab both review and normal comments.

@TylerLeonhardt
Copy link
Member Author

but then again....... commit comments show up in pull requests on the website... so then a user might expect those to come back as well

but I don't agree with that.

@HowardWolosky
Copy link
Contributor

Do you think there's a scenario when you want only Pull Request review comments and not Pull Request normal comments?

I can imagine scenarios where you'd just want PR comments and not review comments. I could see that resolved possibly with a couple parameter sets (these are just the pull request-related ones):

  1. List all comments
  • [int64] PullRequest
  • [switch] $IncludeReviewComments
  1. Get a specific comment
  • [int64] PullRequest
  • [int64] Comment

Keep in mind that this is just for Get-GitHubComment. They would look different in the type-specific functions.
Also keep in mind that proceeding this way means that Get-GitHubIssueComment should no longer be aliased to Get-GitHubComment and instead a separate Get-GitHubComment should be created that starts to support these additional comment types.

@TylerLeonhardt
Copy link
Member Author

I can imagine scenarios where you'd just want PR comments and not review comments

Could follow Get-GitHubPullRequest's State parameter which has validate set of:

All     Closed  Open

could do:

All     ReviewComments  GeneralComments

or something like that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-reactions Work to complete the API's defined here: https://developer.github.com/v3/reactions discussion We are looking for additional community feedback on this topic before proceeding further. enhancement An issue or pull request introducing new functionality to the project. triage needed An issue that needs to be reviewed by a member of the team.
Projects
None yet
Development

No branches or pull requests

2 participants