Allow the ability to reject force pushes to branches with an open pull request

[InExtremaRes]

Feature request

Allow the possibility of reject force pushes to every branch that has an open pull request. This is similar to current branch restriction support but for any branch, not only branches specifically selected.

I imagine right now just a checkbox to enable/disable the ability to force push to a branch with an open PR, but I'm sure there may be more sophisticated patterns.

EDIT Just to clarify and give more context, my rationale for this is that changes in the history of a PR can make the reviewing process harder, since you can no longer trust that previously reviewed commits are still good after the force push. It is possible for an intermediate commit to become buggy even if the head is clean, making bisect and related tools more difficult. Having the possibility to reject force pushes helps to prevent those cases.

Some options that can make this feature more useful is the ability to whitelist users to bypass the restriction, user that ideally could be configured per PR. Also, an option to disable the restriction for specifics PRs may become handy (but is interesting who can do that: Admins? The author? The assigned reviewer?).

Besides, if this feature is enabled, I think merges that modifies the history but triggered from the PR itself should still be allowed (squash & merge, rebase & merge, etc), provided those options are enabled of course.

[6543]

this sounds like reinvent the branch-protection for the whole repo again. wouldn't make it more sence to enhance the branch protection settings more and allow pattern to select branches. this way you clould add a roule "*" witch affect all

[InExtremaRes]

@6543 At first I also thought that way and I saw #2529, but then realize it's not the same I'm asking here. For my use case I don't need to protect every branch, only those with an open PR. We usually push branches we are working on without open a PR so other devs can see what is currently going on and as a measure of backup as well. At that stage the branches are unstable and they can be rebased freely. Then a PR is opened and the branch should be more stable, the reviewing process is harder if the previous commits can change freely; requested changes should be added as new commits, not modifying old ones. There is, of course, the possibility of a final rebase/squash/whatever before closing. Also the reviewer can ask for a rebase or squash to be performed, but he's expecting that, not a surprise.

Is feasible for us to accommodate our workflow if something like #2529 is released? Probably. But since the feature is not exactly the same and is not fully covered I opened a new issue asking for it. Do you have a suggestion of how we can use pattern matching to help the workflow described?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.

[zeripath]

I suspect the correct approach would be instead to offer/suggest to repository admins to branch protect target branches of PRs, inform PR openers when they open a PR that the target is not branch protected, and offer to admins remove branch protection after the pr is merged or closed. The problem is that the powers to add branch protection is separate from merger so people who merge may not be able to remove it - I guess it could have a flag - remove once all PRs are merged?

I think automatically branch protecting on opening any pr could be annoying and make it possible to perform a denial of service on a repository.

Another option is whether we could prevent force pushes to all branches in a repository - that could be a reasonably simple thing to offer.

[jupe]

I would like also something like this. But what about option if opened PR could be branch protected from force pushes after first review comments. Currently it's very annoying that review comments are kind of lost if somebody force push branch and you have to spend time for reviewing whole PR again. Original propose could be also suitable if open PR could be protected unless it's draft. Side effect is that people could workaround restrictions by reverting PR back to draft - force push and open it again. Maybe that's not an issue.

[davidchaine]

Since rebasing and/or amending commits in a published PR is very bad practice, for a couple reasons, it would make a lot of sense to enforce proper behavior and not allow force pushes in branches with published PRs! (Except perhaps draft PRs?)

Atlassian provides a good nutshell explanation too: "If you use pull requests as part of your code review process, you need to avoid using git rebase after creating the pull request. As soon as you make the pull request, other developers will be looking at your commits, which means that it’s a public branch. Re-writing its history will make it impossible for Git and your teammates to track any follow-up commits added to the feature. Any changes from other developers need to be incorporated with git merge instead of git rebase." https://www.atlassian.com/git/tutorials/merging-vs-rebasing

And if running continuous testing through your development, and all commits' tests run green until you publish your PR, but then you rebase, you throw away all your known/confirmed good commits and cannot return to of them later on (e.g. as part of some history/debugging task) and expect it to still be in OK shape.

[rossberg]

As somebody who regularly has to deal with reviewing large PRs where people force-push changes because they don't know better, I confirm this is a serious concern. I wish Github either rejected force-push after comments have been made on a PR, or improved its code review logic to be able to still show diffs and comment history after force-pushes.

[jmanian]

I'm also interested in this feature, for all the reasons above. The new(?) rulesets feature gets extremely close to being able to do this.

All that's really needed is a branch target criteria for "Include branches with open PRs":