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

Can no longer land complaining about no approving review even though reviewers approved it #163

Open
opqpop opened this issue Apr 26, 2023 · 2 comments

Comments

@opqpop
Copy link

opqpop commented Apr 26, 2023

This has just started happening 2-3 days ago and I'm not sure why. Is it possible it has to do with the recent commits that happened since 3 days ago? Pinging the authors too in case it might be related (#159 cc @andrewhamon, #153 cc @cadolphs, #162 cc @spacedentist)

Repro:

  1. git checkout -b branch
  2. make change, git commit (hash: 123)
  3. spr diff, PR is created
  4. have someone stamp it
  5. go to main and get latest changes to prepare for rebase: git checkout origin/main; git pull origin main --rebase
  6. cherry-pick the commit to land git cherry-pick 123
  7. spr diff -m "rebase"
  8. spr land, get below error
> spr land                                          
847b2ea [drac] fix up add wallet (1/n)
  #️⃣   Pull Request #1789
  🛫  Getting started...
  ❌  GitHub Pull Request merge failed
  🛑  GitHub: At least 1 approving review is required by reviewers with write access.
      Documentation URL: https://docs.github.com/articles/about-protected-branches

Let me know if there's anything else I can help with

@opqpop
Copy link
Author

opqpop commented May 1, 2023

Hmm it might be because we've recently added pre-commit hooks that require an approval before it can be merged, as well as passing a build check

but my approval + build check is all against the parent diff instead of the main branch, so when spr land swaps it to be against the main branch, it fails the pre-commit hook until the newly build check that got kicked off finishes.

Any suggestions for how to best get around this? Is there a way for spr to bypass pre-commit hooks vs main, as long as they passed when it was against the parent diff?

@dukeofcool199
Copy link

we ran into a similar issue recently, ours occured in a flow like this.

  1. commit A onto main
  2. spr diff
  3. commit B onto main
  4. spr diff
  5. B gets review with "changes requested"
  6. B requested changes get made
  7. B and A get re-ordered so that B can be merged first
  8. B gets accepted
  9. B cannot be landed because it claims it is not accepted

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

No branches or pull requests

2 participants