Skip to content

Bedevere rejects change that targets another PR #381

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

Closed
jaraco opened this issue Dec 16, 2021 · 5 comments · Fixed by #470
Closed

Bedevere rejects change that targets another PR #381

jaraco opened this issue Dec 16, 2021 · 5 comments · Fixed by #470

Comments

@jaraco
Copy link
Member

jaraco commented Dec 16, 2021

In python/cpython#30151, I've written a PR that's based on another PR. In order for the diff to look right but for the code to be based on another PR, I've set the target for the PR as the other PR:

image

Github supports this workflow and even has some nice features around it. In particular, when python/cpython#30150 is merged, Github will automatically detect the merge and will re-point the 30151 to point to main.

Unfortunately, this workflow trips up on bedevere's check for maintenance branch PR naming, causing the tests not to run:

image

It would be nice if bedevere would not enforce the naming check if it's not pointing at a maintenance branch. Even better would be for it to recognize that it's pointing at a branch that's pending merge to main or a maintenance branch and use that resolution to make its check.

Impact due to this issue is small, and I'm not even sure there's enough value in fixing the issue, but I'd like to register it regardless.

@jaraco
Copy link
Member Author

jaraco commented Dec 16, 2021

For this particular case, I found I was able to actually rebase the dependent change, making it independent, and target it at main separately, working around the issue. This would not have been possible if there had been a hard dependency on the first PR.

@arhadthedev
Copy link
Member

Is the issue relevant after the migration?

@ezio-melotti
Copy link
Member

Since the issue seems to be about PRs based on other PRs, the migration doesn't affect this issue. We should verify if the issue is still present or if it got fixed in the meanwhile before closing this.

@jaraco
Copy link
Member Author

jaraco commented Jul 3, 2022

I wanted to confirm, this change appears to be working, as is evidenced by python/cpython#94528, which targets python/cpython#93965.

@Mariatta
Copy link
Member

Mariatta commented Jul 3, 2022

Awesome. Thank you!

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

4 participants