-
Notifications
You must be signed in to change notification settings - Fork 518
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
Feature: auto rebase of PRs #1358
Comments
their code is mostly a shell script and a docker files that pulls a few utilities https://github.com/cirrus-actions/rebase/blob/master/Dockerfile#L12 |
file https://github.com/cirrus-actions/rebase/blob/master/entrypoint.sh looks ok. There are a few places where it does not put env variable between "", which is also caught when I used https://www.shellcheck.net/ on their code. |
@laurentsimon I wonder why PRs would have to be rebased in the first place? Considering that PRs are usually reviewed almost as soon as they are opened I think as long as they don't conflict with the main branch it should be fine to merge them even if GitHub says that they are out of date. If it isn't clear whether PRs can break something it's always possible to ask contributors to force-push their branches to retrigger the tests and let GHActions rebase/merge them on top of the main branch to make sure the CI is green. |
Looking at https://github.com/marketplace/actions/automatic-rebase it seems for it to work it has to change contributors' repositories and personally I wouldn't be happy if bots rebased branches in my repositories :-) Another issue is that contributors can flip the "Allow edits and access to secrets by maintainers" flag to prevent changes like that. |
This issue is stale because it has been open for 60 days with no activity. |
We spend too much time rebasing PRs to kick off pre-submit runs. Especially when we have many PRs open, it's a waste of time.
It would be great to be able to auto-rebase PRs automatically, so that once we've clicked "auto-merge" button, the PR gets merged automatically when they can (of course, LGTM should still be enforced).
I found this https://github.com/marketplace/actions/automatic-rebase which seems encouraging. I wonder if this requires dangerous write permissions... since it needs to push to remote cloned branch...?
Answering my own questions: cirrus-actions/rebase#84 they need pull-request:write` and this is a danger in itself since it allows changing the code before we merge :/
Probably we need to review the code and pin to this hash for the code we've looked at.
Do we loose other important things by using an auto PR rebase feature?
The text was updated successfully, but these errors were encountered: