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

Fix duplicated GitHub action runs #398

Merged
merged 1 commit into from
Feb 19, 2021

Conversation

dsander
Copy link
Contributor

@dsander dsander commented Feb 19, 2021

It is not recommended to use pull_request_target when code is executed
that could modify the repository.

It is not recommended to use `pull_request_target` when code is executed
that could modify the repository.
@dsander
Copy link
Contributor Author

dsander commented Feb 19, 2021

When this is merged pronto will probably not be able to comment on PR anymore because it does not have the right permissions. This is really difficult to test because the actions seem to behave differently depending on the repository they are running from/on. The same PR on my fork passes CI: dsander#2

I think what we want is something like the setup described here (search for CommentPR.yml). Do you know how the commenting worked with TravisCI? Was there a GitHub token in the CI secrets?

@dsander dsander marked this pull request as ready for review February 19, 2021 13:52
@dsander dsander requested a review from a team as a code owner February 19, 2021 13:52
@ashkulz
Copy link
Member

ashkulz commented Feb 19, 2021

It's fine if we remove pull_request_target -- but why is the pull_request failing for Ruby 2.4? It happens on this PR as well.

@ashkulz
Copy link
Member

ashkulz commented Feb 19, 2021

I created #399 which still ran the duplicated runs, but weirdly enough it was successful for 2.4 🤷‍♂️

@ashkulz ashkulz merged commit 80c88b7 into prontolabs:master Feb 19, 2021
@dsander
Copy link
Contributor Author

dsander commented Feb 19, 2021

I think that is/was because depending on the trigger and origin of the PR (either a fork or on this repository) the commits are checked out differently, but I am not 100% sure.

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

Successfully merging this pull request may close these issues.

None yet

2 participants