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.

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

Fix for changelog verifier and milestone setter automation #14369

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pseudo-nymous
Copy link
Contributor

@pseudo-nymous pseudo-nymous commented Mar 19, 2025

Description

This pull request contains a fix for changelog automation that has been added recently. We have seen failures where either diff calculation wrt base commit was wrong or base commit was not even available leading to fatal error.

This happens because we are using pull_request_target event type which checks out merge base commit where as we want to checkout head commit to do proper diff calculation wrt changelog file. My initial testing was based on pull_request which worked as expected since it checkouts merge head commit.

We tried a fix for this here: #14355. But we didn't fetch the full history appropriately. This PR adds a fix for this by pulling all the associated history using git pull --unshallow origin ${{ github.event.pull_request.head.ref }}.

Tested all the changes here: pseudo-nymous#22

Addresses: #13898 #14190

@pseudo-nymous
Copy link
Contributor Author

We can fetch all history using checkout actions itself using flag fetch-depth: 0. But it fetches all the history for all branches and tags which is not required here.

@stefanvodita
Copy link
Contributor

I'm happy to try this out, but I have some doubts it addresses the issue we're experiencing now. For example, take the failure here.

In step 1, we Could not resolve to a PullRequest with the number of 14301. The PR definitely exists.
Then, in step 2 we get fatal: bad object. The fix here is meant to fix that, right? But I wonder if the issue goes deeper.

What do you think @pseudo-nymous?

@pseudo-nymous
Copy link
Contributor Author

pseudo-nymous commented Mar 19, 2025

Yes, fix here would fix the fatal: bad object failue. I have tested the changes by merging my fix onto my fork and everything worked as expected.

I haven't seen the first failure before (Could not resolve to a PullRequest with the number of 14301.), let me address this.

@pseudo-nymous pseudo-nymous marked this pull request as draft March 20, 2025 03:39
@pseudo-nymous
Copy link
Contributor Author

Moved it to draft state to address all the failures first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants