Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
earl-warren opened this issue Mar 31, 2024 · 7 comments · Fixed by #118
Closed

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

earl-warren opened this issue Mar 31, 2024 · 7 comments · Fixed by #118
Assignees
@earl-warren

Comments

@earl-warren
Copy link
Contributor

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
Copy link
Member

lampajr commented Mar 31, 2024

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:

  • backport, which uses the default no-squash
  • backport-squash, to change the default behavior

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

@earl-warren
Copy link
Contributor Author

That is a nice workaround 👍

Do you know something more?

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.

@lampajr
Copy link
Member

lampajr commented Mar 31, 2024 •
edited
Loading

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:

  • If merged as a merge commit, merge_commit_sha represents the SHA of the merge commit.
  • If merged via a squash, merge_commit_sha represents the SHA of the squashed commit on the base branch.
  • If rebased, merge_commit_sha represents the commit that the base branch was updated to.

"""

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 👀

@earl-warren
Copy link
Contributor Author

earl-warren commented Mar 31, 2024 •
edited
Loading

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?

@lampajr
Copy link
Member

lampajr commented Apr 2, 2024 •
edited
Loading

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 earl-warren mentioned this issue Apr 2, 2024
Merged
@earl-warren
Copy link
Contributor Author

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
Copy link
Member

lampajr commented Apr 2, 2024

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 😄

earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 3, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
bb70a5b 
Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 3, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
585d522 
Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 3, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
b2554a6 
Fixes: kiegroup#113
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this issue Apr 3, 2024
@earl-warren
[CI] backport auto-squash detection and -x
6c71793 
* Behaves as it should with merge & squashed pull requests
* Cherry-pick commits with -x for traceability

Refs: kiegroup/git-backporting#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 5, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
7510e7d 
Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 5, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
d74b7e3 
The auto-no-squash option is added to:

* backport all the commits when the pull request / merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull request / merge request.

Detecting if a pull request / merge request was squashed or not
depends on the underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull request / merge request is open, always backport all the
commits it contains.

Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 8, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
8d22eaa 
The auto-no-squash option is added to:

* backport all the commits when the pull request / merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull request / merge request.

Detecting if a pull request / merge request was squashed or not
depends on the underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull request / merge request is open, always backport all the
commits it contains.

Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 8, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
f00cec9 
The auto-no-squash option is added to:

* backport all the commits when the pull/merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull/merge request request.

Detecting if a pull/merge request was squashed or not depends on the
underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull/merge request is open, always backport all the commits it
contains.

Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 8, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
bcefd52 
The auto-no-squash option is added to:

* backport all the commits when the pull/merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull/merge request request.

Detecting if a pull/merge request was squashed or not depends on the
underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull/merge request is open, always backport all the commits it
contains.

Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 8, 2024
@earl-warren
feat: auto-detect the value of the no-squash option
44184ee 
The auto-no-squash option is added to:

* backport all the commits when the pull/merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull/merge request request.

Detecting if a pull/merge request was squashed or not depends on the
underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull/merge request is open, always backport all the commits it
contains.

Fixes: kiegroup#113
earl-warren added a commit to earl-warren/git-backporting that referenced this issue Apr 8, 2024
@earl-warren @lampajr
feat: auto-detect the value of the no-squash option
f591021 
The auto-no-squash option is added to:

* backport all the commits when the pull/merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull/merge request request.

Detecting if a pull/merge request was squashed or not depends on the
underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull/merge request is open, always backport all the commits it
contains.

Fixes: kiegroup#113

Co-authored-by: Andrea Lamparelli <a.lamparelli95@gmail.com>
lampajr added a commit that referenced this issue Apr 8, 2024
@earl-warren @lampajr
feat: auto-detect the value of the no-squash option (#118)
6042bcc 
The auto-no-squash option is added to:

* backport all the commits when the pull/merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull/merge request request.

Detecting if a pull/merge request was squashed or not depends on the
underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull/merge request is open, always backport all the commits it
contains.

Fixes: #113

Co-authored-by: Andrea Lamparelli <a.lamparelli95@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@earl-warren earl-warren

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Auto-detect the value of the no-squash option earl-warren/git-backporting
2 participants
@lampajr @earl-warren