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

feat: support PR comments for push events #390

Merged
merged 2 commits into from Aug 7, 2022
Merged

feat: support PR comments for push events #390

merged 2 commits into from Aug 7, 2022

Conversation

micnncim
Copy link
Contributor

@micnncim micnncim commented Aug 6, 2022

Since GitHub no longer shows commit comments in the pull request timeline 1, it needs to find a PR associated with a commit in a push event and then post a PR comment if found in order to show comments in the pull request timeline.

See #387 for more details.

Footnotes

  1. https://github.blog/changelog/2022-08-04-commit-comments-no-longer-appear-in-the-pull-request-timeline

Since GitHub no longer shows commit comments in the pull request
timeline [^1], it needs to find a PR associated with a commit in a push
event and then post a PR comment if found in order to show comments in
the pull request timeline.

[^1]:
github.blog/changelog/2022-08-04-commit-comments-no-longer-appear-in-the-pull-request-timeline

Signed-off-by: micnncim <micnncim@gmail.com>
@micnncim
Copy link
Contributor Author

micnncim commented Aug 6, 2022

@suzuki-shunsuke

@suzuki-shunsuke
Copy link
Owner

Thank you for your contribution!

@suzuki-shunsuke suzuki-shunsuke added the enhancement New feature or request label Aug 6, 2022
prs, _, err := g.client.API.PullRequestsListPullRequestsWithCommit(ctx, sha, nil)
if err != nil {
return 0, err
}
for _, pr := range prs {
if pr.GetState() != "closed" {
if pr.GetState() != string(state) {
Copy link
Owner

Choose a reason for hiding this comment

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

Could you check Pull Request's UpdatedAt?

How to handle when multiple pull requests are associated
Choose the latest updated pr

#387 (comment)

I don't know the order of prs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: micnncim <micnncim@gmail.com>
Copy link
Owner

@suzuki-shunsuke suzuki-shunsuke left a comment

Choose a reason for hiding this comment

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

Thanks!

@suzuki-shunsuke suzuki-shunsuke merged commit e1df4c8 into suzuki-shunsuke:main Aug 7, 2022
@suzuki-shunsuke suzuki-shunsuke added this to the v3.4.0 milestone Aug 7, 2022
@micnncim micnncim deleted the commit-comments branch August 7, 2022 09:34
@micnncim
Copy link
Contributor Author

micnncim commented Aug 7, 2022

@suzuki-shunsuke Thanks for your quick review! When are planning to release a new version?

@micnncim micnncim restored the commit-comments branch August 7, 2022 09:35
@suzuki-shunsuke
Copy link
Owner

I have already released v3.4.0.
https://github.com/suzuki-shunsuke/tfcmt/releases/tag/v3.4.0

@micnncim
Copy link
Contributor Author

micnncim commented Aug 7, 2022

I missed the release. Thanks a lot again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants