search

telia-oss / github-pr-resource

Github pull request resource for Concourse
MIT License
182 stars 5 forks source link

Possible for `check` to introduce new versions when the PR body is updated? #165

Closed rnag closed 4 years ago

rnag commented 4 years ago

Hello! I have a use case where I want to re-trigger status checks on the latest commit in a PR whenever a user edits the PR body - this is mainly since we're doing some validation on the contents of the PR notes.

I looked into the pull-request event trigger on GitHub, and it seems to state that it will correctly send an event to the webhook whenever the PR is edited: image

I also see that the webhook payload is being correctly sent: image

However, i was just curious if there's a way to get the PR resource to report a new version on "check" whenever the PR body is updated. I'm not entirely sure, but it seems like the easiest way would be to check if the "action" field sent in the webhook payload is equal to edited . In this case, ideally i would want to trigger a Concourse task to run to validate the updated PR notes.

So assuming this would be easy enough to do, would this be a feature worth supporting? I'm not entirely sure, but It seems like the current pr-resource only checks and reports if there are any new commits made to an open PR - so just curious what the thoughts would be on this.

itsdalmo commented 4 years ago

Hi and thanks for submitting an issue!

Unfortunately, Concourse does not pass the webhook payloads to resources and instead just triggers are regular check when triggered by a webhook. As such, I don't think there is a good way to implement what you suggest without including e.g. lastEditedAt for the pull request in the version history, in order to be able to compare the previous and current value and determine whether it's a "new version".

Given the above, and because I prefer to keep the version object as lean as possible, I think your best option might be to implement this as a separate resource?

rnag commented 4 years ago

Oh ok.. no problem! Thanks for clarifying though - I was suspecting as much but I wasn't sure whether the webhooks payloads were passed to the resource at all.

I appreciate the suggestion about the lastEditedAt - i will look into whether i can implement it as a separate resource. Thanks again! I think this issue can be closed for now, for the same reasons that you mentioned 👍