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

Trigger apply run from "digger apply" on PR review comments #1220

Open
noralutz opened this issue Feb 28, 2024 · 4 comments
Open

Trigger apply run from "digger apply" on PR review comments #1220

noralutz opened this issue Feb 28, 2024 · 4 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@noralutz
Copy link

Currently, this comment does not cause Digger to react, meaning I have to make another comment after I approve.

@motatoes
Copy link
Contributor

motatoes commented Mar 4, 2024

Interesting, looks like that triggers a pull_request_review_comment event :D . We need to handle that in orchestrator and handle it as a comment event 👍 Adding as good first issue to see if someone wants to grab it

@motatoes motatoes added good first issue Good for newcomers bug Something isn't working labels Mar 4, 2024
@ahuseby
Copy link

ahuseby commented Apr 16, 2024

This would be a good addition indeed.

An important difference between issue_comment and pull_request_review_comment is that GITHUB_REF is the default branch for the former, but refs/pull/PULL_REQUEST_NUMBER/merge for the latter (see docs). This makes the review comment more integrated into the pull request context (e.g. not having to do this).

A GitHub feature request has been raised for making issue_comment in PRs (or a new PR comment event type) use the PR ref, but until then the pull_request_review_comment is the best option.

@motatoes
Copy link
Contributor

Thanks @ahuseby ! So we will need to that event and handle it on digger side. I think issue number will still remain so that action.yml logic would still work

@ahuseby
Copy link

ahuseby commented Apr 16, 2024

Thanks @ahuseby ! So we will need to that event and handle it on digger side. I think issue number will still remain so that action.yml logic would still work

That's right 😄 I'll try and take a look at contributing a change for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants