Poor interaction with force-pushed branches about create-pull-request HOT 4 CLOSED

Great! Glad that worked for you.

Agreed. It might be nice to expose functionality to rebase the entire PR branch after resolving conflicts on the tip of the base branch, though.

I'll look into adding this back as an optional feature.

Just found the -multi version that addresses this well enough for our use-case.

I'm not sure what direction GitHub are going to take yet regarding multi-platform Actions support so I'm holding off making the -multi version the main version of this action. There is a bit of background here: #40 (comment)
You are right though, the container version startup time could be improved by pushing it to DockerHub. I might do that regardless because someone could find it useful to run outside of GitHub Actions.

from create-pull-request.

Glad you find this action useful. Thank you for raising this issue.

When using the fixed-name branch strategy (BRANCH_SUFFIX: none) the action needs to check out the remote branch to update it. I couldn't check out the branch without committing local changes first so it does a git rebase afterwards. What I imagine is happening is that during the rebase (and because of force-pushed changes) cherry-picked commits resolve to an empty diff and throws the above warning. I'll try to reproduce this but still not sure exactly what "force-pushed to something else" means in practice.

One solution might be to use the --keep-empty rebase option to allow empty commits to be made. Another solution might be to stop rebasing and instead git stash the local changes, checkout the branch, and then git stash pop to apply the local changes on top of the branch. If that pop resolves to an empty diff then the action can just exit without doing anything.

from create-pull-request.

@Xyene I've refactored the fixed-name branch strategy to not use rebase anymore. Having the branch rebased might be convenient in some cases, but I think it would be best to avoid using it by default because users of this action won't expect that.

So instead, when using fixed-name branch strategy the action does this:

1. Stash the local changes
2. Checkout the remote branch
3. Pop the stash
4. Resolve conflicts (if any) in favour of the stashed version

I think this method should prevent the kind of error you saw occur because if there are no changes to the branch after the stash is popped the action will just exit silently. If there are conflicts, they should all be resolved in favour of the stashed version, which is most likely correct.

Please test out release peter-evans/[email protected] and let me know if it works for you.

from create-pull-request.

@peter-evans thanks, that seems to have worked well!

I'll try to reproduce this but still not sure exactly what "force-pushed to something else" means in practice.

In our case, it was a workflow that involves rebases that change history (e.g. renaming a commit to something more descriptive, or reordering commits to make more "sense" when reading the git log) on a feature branch, in order to keep the master log as clean as possible.

Having the branch rebased might be convenient in some cases, but I think it would be best to avoid using it by default because users of this action won't expect that.

Agreed. It might be nice to expose functionality to rebase the entire PR branch after resolving conflicts on the tip of the base branch, though. We immediately ran into a small issue where another action (using the workaround from #48) wasn't being triggered on the PR, since it was added in a later commit than where the branch was created. Of course, this just required a button press in Github's UI, but is one that would need to be constantly pressed by a human, who would then need to wait for checks to pass.

Unrelated, but what are your thoughts on uploading the built Docker image to Docker Hub? Currently it takes ~1 minute to build from scratch, which isn't that bad, but probably won't be getting any faster. (I guess there could be a Github Action for automatically pushing updates to Docker hub... meta ;) Just found the -multi version that addresses this well enough for our use-case.

I'd be happy to contribute either of the above proposals if they're something you think would generally be useful, but I'll close this issue as it's now resolved 😃

from create-pull-request.