-
Notifications
You must be signed in to change notification settings - Fork 62
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
Check PR Author instead of Action Actor #137
Conversation
closes issue dependabot#112
@@ -6,7 +6,7 @@ permissions: | |||
jobs: | |||
dependabot: | |||
runs-on: ubuntu-latest | |||
if: ${{ github.actor == 'dependabot[bot]' }} | |||
if: ${{ github.event.pull_request.user.login == 'dependabot[bot]' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ Thanks for updating the documentation as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mwaddell! This change looks good to me.
I adjusted my workflow to take advantage of this change, but now it fails if I add commits to to the PR. Typical reasons for this would be to add an entry to the changelog, fix linting rules, or as happened today fix an incorrect version update (a couple actions used What's the best way to work around this? Maybe a new output indicating if non-dependabot commits were found so you could skip subsequent steps? |
@brrygrdn What's the reason for failing the run if the PR contains commits after the original dependabot one? Is there a good reason not to delete https://github.com/dependabot/fetch-metadata/blob/main/src/dependabot/verified_commits.ts#L35:L38 |
@brrygrdn - I added #166 to remove those lines in order to address the issue noted by @xt0rted - let me know if there's a reason we can't do this. |
That's a good question, it's something I've actually been blocked by myself this week. I originally added it as we were (defensively) trying to facilitate narrowest possible definition of a Dependabot PR, i.e.
I don't think any potential benefit or risk it mitigates really offsets the fact that an extremely common workflow like merging in the target branch is blocked. I'll have a chat with the team on Monday morning to get a second opinion, but I'm leaning towards removing this. |
closes #112