search

kiegroup / git-backporting

Git backporting is a CLI tool to execute pull request backporting.
MIT License
16 stars 9 forks source link

Auto-detect the value of the no-squash option #113

Closed earl-warren closed 7 months ago

earl-warren commented 8 months ago

Some pull requests may be squashed into a single commit and others merged to preserve the commit history. If the value of no-squash was automatically determined, it would be possible for both to be backported. Otherwise backport will fail for the merged pull requests that do not match the value of no-squash.

An alternative is for a step occuring before git-backporting to determine the value of no-squash.

This happens in the https://codeberg.org/forgejo/forgejo/ repository where all pull requests are merged without squashing, except for the translations.

lampajr commented 8 months ago

Hi @earl-warren, that could be an interesting improvement but AFAIK, using the Github API, there is no reliable way to check whether a pull request has been merged with squash, rebase&merge or merge-commit. Do you know something more?

An alternative is for a step occuring before git-backporting to determine the value of no-squash.

That's exactly an approach we are using here where we manage two different label patterns:

Obviously this has the drawback that whoever add the label MUST know which one to choose

earl-warren commented 8 months ago

That is a nice workaround :+1:

Do you know something more?

I did not look into it :blush: What about github.event.pull_request.merge_commit_sha : if it is a merge commit, then it means the PR was merged. Otherwise it means it was not.

lampajr commented 8 months ago

I did not look into it 😊 What about github.event.pull_request.merge_commit_sha : if it is a merge commit, then it means the PR was merged. Otherwise it means it was not.

That's not that easy as it seems, based on the documentation:

""" The value of the merge_commit_sha attribute changes depending on the state of the pull request. Before merging a pull request, the merge_commit_sha attribute holds the SHA of the test merge commit. After merging a pull request, the merge_commit_sha attribute changes depending on how you merged the pull request:

"""

This means that the merge_commit_sha is quite always populated but its meaning varies based on the pull request status or merge condition.

Anyway I can deep dive in the documentation if I can find something useful :eyes:

earl-warren commented 8 months ago

I think that there is a simple and useful case. The default of no-squash could be set depending on whether the merge_commit_sha is a merge commit or not instead of always be no-squash: false.

It would be backward compatible because git-backporting will always fail if merge_commit_sha is a merge commit and no-squash defaults to false because it is not specified. It is not a useful default in that case.

Does that make sense or am I missing something?

lampajr commented 8 months ago

I think that there is a simple and useful case. The default of no-squash could be set depending on whether the merge_commit_sha is a merge commit or not instead of always be no-squash: false.

  • If merge_commit_sha is a merge commit no-squash: true by default.
  • If merge_commit_sha is not a merge commit no-squash: false by default.

It would be backward compatible because git-backporting will always fail if merge_commit_sha is a merge commit and no-squash defaults to false because it is not specified. It is not a useful default in that case.

Does that make sense or am I missing something?

That would makes sense but how do you know whether merge_commit_sha is a merge commit or not? (given that once the PR is merged it is always filled in) I mean using the api or the pull request object?

earl-warren commented 8 months ago

This draft https://github.com/kiegroup/git-backporting/pull/118/files explains what I have in mind, roughly. I did not go into the fine details yet and maybe there is something I'm missing and it won't work?

lampajr commented 8 months ago

This draft https://github.com/kiegroup/git-backporting/pull/118/files explains what I have in mind, roughly. I did not go into the fine details yet and maybe there is something I'm missing and it won't work?

Commented there, overall I agree on the appraoch but there is still one thing I do not have clear: "how can we infer if the merge_commit_sha is a merge commit or not" but I might be missing something to :smile: