Merge Dependabot PRs instead of closing

[ppkarwasz]

This PR changes the way Dependabot PR's appear in a repository history.

Currently their changes are applied only to the base branch and the PR itself is closed.

Instead of closing the PR we can reset its branch to the updated base branch. Github will detect this change and mark the PR as "merged" instead of "closed".

This technique has been tested on my personal repo:

https://github.com/copernik-eu/log4j-plugins/actions/runs/7219181183

[ppkarwasz]

I installed this on logging-log4j-samples for testing.

[vy]

This PR changes the way Dependabot PR's appear in a repository history.

Currently their changes are applied only to the base branch and the PR itself is closed.

Instead of closing the PR we can reset its branch to the updated base branch. Github will detect this change and mark the PR as "merged" instead of "closed".

Okay – PR will appear as merged instead of closed with a comment. What is the advantage of this outcome?

I am reluctant to go forward with this. Since basic git push, git fetch, etc. commands used on a daily basis are replaced with more advanced ones with complicated references, of which I doubt anybody else in the PMC will understand except you.

[ppkarwasz]

Okay – PR will appear as merged instead of closed with a comment. What is the advantage of this outcome?

I am reluctant to go forward with this. Since basic git push, git fetch, etc. commands used on a daily basis are replaced with more advanced ones with complicated references, of which I doubt anybody else in the PMC will understand except you.

I admit that closed PRs look bad in my notifications and a repository Insights. Not sure if there are other advantages.

Are these git commands really so obscure? I commented them to specify what we want to obtain.

[vy]

I admit that closed PRs look bad in my notifications and a repository Insights. Not sure if there are other advantages.

PR complicates the existing code, we need stronger arguments than these, IMHO.

Are these git commands really so obscure? I commented them to specify what we want to obtain.

Yes, let me give concrete examples:

# Must include the merge-base between head and target branch.
fetch-depth: 32

I have no idea what the comment tries to mean and why the magical 32.

# We only fetched the PR branch, we need to fetch the target branch
git fetch origin "+refs/heads/$BASE:refs/remotes/origin/$BASE"

I don't know of a single PMC member who would understand this without reading a couple of git manpages and SO posts. If you think there are, please tell them to join us in this conversation.

Can I suggest a different route: Can't you cherry-pick the dependabot commit (AFAIK, GitHub will recognize this and mark the PR as merged) and update the changelog in a separate commit?

[ppkarwasz]

Can I suggest a different route: Can't you cherry-pick the dependabot commit (AFAIK, GitHub will recognize this and mark the PR as merged) and update the changelog in a separate commit?

I can test if it works.

[ppkarwasz]

@vy,

I have completely rewritten the PR. Now it only pushes the same commit to main and the dependabot/* branch in order for Github to mark it as merged.

When dealing with a lot of Dependabot PRs the difference between "closed" and "merged" is very useful. E.g. sometimes Dependabot also closes PRs, when a new version of the dependency is released. This change should help differentiating the two cases.

[vy]

Thanks for chasing this. I am really happy with the state we reached:

1. It is less LoC
2. It is still simple (i.e., easy to understand)

My only nitpick regret is you replaced wget with curl – I always find the latter to be a CVE-generator and not included in Debian-based distros by default. But if that is your preference, I am fine with it.

In overall, LGTM. You can merge it.

[ppkarwasz]

I tested this in https://github.com/apache/logging-log4j-jakarta/actions/runs/7453139186