search

korthout / backport-action

Fast and flexible GitHub action to cherry-pick merged pull requests to selected branches
MIT License
61 stars 26 forks source link

Feature request: allow specifying mainline #341

Closed houserx-jmcc closed 1 year ago

houserx-jmcc commented 1 year ago

Allow specifying a default mainline to use when cherry picking and if supplied use it, i.e. git cherry-pick 1234567 -m 1. Since this action can be used in very specific workflows (like my use case!), this could be a useful option to specify.

Example:

steps:
      - uses: actions/checkout@v3
      - name: Create backport pull requests
        uses: korthout/backport-action@v1
        with:
           default_mainline: 1
korthout commented 1 year ago

Hi @houserx-jmcc 👋 and thanks for raising this! I ran into a case where I also needed this. The pull requests were updated with the recent changes from the target branch with merge commits.

Could you share you're use-case/workflow? It may help me prioritize this feature.

houserx-jmcc commented 1 year ago

Thanks! We are working through some changes to our development flows, so I wouldn't say we have an explicit path at the moment, but it seems that some way or another we find ourselves often needing to cherry pick upwards or downwards from a merge commit 🙃

korthout commented 1 year ago

With #340 completed and released in 1.3.0, I'm deciding what to implement next. Are you still in need of this feature @houserx-jmcc?

houserx-jmcc commented 1 year ago

A nice to have, but our initial need for it has waned a bit so no need to prioritize it above other things. Thank you for checking in!

tdonohue commented 1 year ago

We've hit this problem ourselves... the backport-action fails anytime a developer updates their PR with one or more merge commits. Here's an example of a very small PR which fails to be backported because the second commit is a merge commit (developer updated their PR to latest code by merging main back into the PR): https://github.com/DSpace/dspace-angular/pull/2381/commits

Because we accept PRs from anyone via developer forks, we cannot stop the developer from updating their PR via a merge commit (or rebase the PR to remove the merge commit). Unfortunately, anytime they do so, this backport-action fails & we have to perform a manual cherry-pick instead.

So, I agree it'd be nice if there was a way for this action to somehow skip (or support) merge commits in a more automated manner!

korthout commented 1 year ago

Thanks for sharing this @tdonohue. It helps me prioritise this issue knowing that more users are running into it. I can't promise when I'll find time for it with it being summer and all, but now this one is next on my list of improvements for backport-action.

korthout commented 1 year ago

A first version of this is available for testing in korthout/backport-action@341-default-mainline, which supports a new default_mainline input.

Note that the only usable values at this time are:

When disabled, the behavior is as before, i.e. the action fails when it encounters a merge commit and reports about its failure. Using 1 as the default mainline means that all changes from the merge commit are cherry-picked to a new commit.

It is not yet possible to use 2 (or higher) as the default_mainline because the mainline flag is applied to the cherry-pick of each commit, incl. any non-merge commits. In this case, the action fails when a commit does not have that many parents. I've tried to keep the changes small and simple for now. I think we can do this in a separate feature request.

It is not yet possible to skip merge commits. That may be a useful behavior for default_mainline: 0. I think we can also split this to a separate feature request.

@tdonohue I'd love to know whether the default_mainline: 1 behavior solves the issue for you, or whether you have an explicit need for any of the other options I described. If the feature works for you as is then I'm happy to release this properly.

tdonohue commented 1 year ago

@korthout : I gave it a try on our end. Unfortunately, the --mainline=1 flag to cherry-pick doesn't seem to work for our scenario. I suspect we need --skip for the merge commits (at least most of them), as they are providing no useful/new code to the backport PR. More details/logs at https://github.com/korthout/backport-action/pull/372#issuecomment-1670270189

korthout commented 1 year ago

Thanks for your feedback @tdonohue. It is very helpful.

I am now quite convinced that the common use case requires skipping the merge commits. We can continue this in a separate issue.

I also think that this feature request is no longer needed, so I'll close this issue. But, if anyone sees a use case for setting a default mainline then please let me know. Feel free to reopen this issue.