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

Docs: Update Example to minimize Approval-notification-spam #138

Closed
fbartho opened this issue Feb 7, 2022 · 10 comments · Fixed by #188
Closed

Docs: Update Example to minimize Approval-notification-spam #138

fbartho opened this issue Feb 7, 2022 · 10 comments · Fixed by #188
Assignees
Labels
enhancement New feature or request

Comments

@fbartho
Copy link

fbartho commented Feb 7, 2022

This action has an example for how to enable auto-approve:

fetch-metadata/README.md

Lines 68 to 72 in ffa0846

- name: Approve a PR
run: gh pr review --approve "$PR_URL"
env:
PR_URL: ${{github.event.pull_request.html_url}}
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}

In our repo and many repos, a previous approval is not invalidated by a rebase. And the default example causes dependabot to auto-approve every time it rebases that PR, this approval appears as an additional email for subscribed viewers. So, every rebase causes an email for the rebase and an email for the approval.

Proposed alternative command:

(gh pr status --json url -q '.needsReview.url?' | grep "$PR_URL" && gh pr review --approve "$PR_URL") || echo "PR already approved, skipping additional approvals to minimize emails/notification noise."

This alternative command would skip the approval if the PR in question isn't present in the needsReview section of the JSON output for gh pr status.

In addition to feedback on this docs update, if my command isn't optimal, I'd love for advice for a better command to use in its place. This requires relatively sophisticated shell-expressions, and I'd love to simplify this, and make it more robust.

@fbartho fbartho added the enhancement New feature or request label Feb 7, 2022
@fbartho
Copy link
Author

fbartho commented Feb 8, 2022

Warning Turns out, that that CLI command I proposed above doesn’t reliably work. So I’m still looking for a less-noisy solution to this problem if anybody has any advice?

@mwaddell
Copy link
Contributor

@fbartho How about something like:

if [ $(gh pr status --json reviewDecision -q .currentBranch.reviewDecision) == "REVIEW_REQUIRED" ]; then 
  gh pr review --approve "$PR_URL"
else
  echo "PR already approved, skipping additional approvals to minimize emails/notification noise."
fi

@fbartho
Copy link
Author

fbartho commented Feb 20, 2022

Thanks @mwaddell! That looks great. I’ll try that out.

@mwaddell
Copy link
Contributor

mwaddell commented Mar 5, 2022

@fbartho - let me know if that worked, if so, I'll update the example in our documentation.

@fbartho

This comment was marked as outdated.

@fbartho
Copy link
Author

fbartho commented Mar 7, 2022

@mwaddell Actually, I spoke too soon. Looks like a bash error?

image

@mwaddell
Copy link
Contributor

I looked into it a bit more, and it looks like this approach won't work until they fix cli/cli#575

👎

@fbartho
Copy link
Author

fbartho commented Mar 12, 2022

Bummer! Could I ensure our upstream is set for the repo or otherwise configure it for the purposes of this API call?

@mwaddell
Copy link
Contributor

Bummer! Could I ensure our upstream is set for the repo or otherwise configure it for the purposes of this API call?

I think I figured out a workaround - see if #188 works for you. It looks a little silly to checkout the repo and then check it out again, but that's the only way to force the upstream to be set properly.

@fbartho
Copy link
Author

fbartho commented Mar 30, 2022

I can confirm that #188 fixed this issue for us! -- It took a moment to get new Dependabot PRs to verify everything was working!

Obviously we'd still prefer a technique that doesn't require checking out the repo twice, but this is a good workaround for now.

Thanks!

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 a pull request may close this issue.

3 participants