Skip to content

Fix missing release branch when looking for commits #1055

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

Merged

Conversation

sdb9696
Copy link
Contributor

@sdb9696 sdb9696 commented Oct 30, 2024

The default responses per page of 30 can cause the release branch to be missed out of the query results during the check for commits in the release branch.

This causes WARNING: merge commit was not found in the release branch or tagged git history and no rebased SHA comment was found. #971

This PR sets the default query response size to 100 to reduce the likelihood of this happening. A more permanent solution could be to restrict the branches query to the required branch.

@olleolleolle
Copy link
Collaborator

Right, 30 branches lying around is nothing. Even 100 could be very little. But offering the default options to the API request should be uncontroversial.

@sdb9696
Copy link
Contributor Author

sdb9696 commented Oct 31, 2024

The query also brings back deleted branches which doesn’t help. I’ll look shortly to see if there’s a parameter to control that.

@sdb9696
Copy link
Contributor Author

sdb9696 commented Oct 31, 2024

The latest commit iterates the pages so no 100 limit either.

@sdb9696
Copy link
Contributor Author

sdb9696 commented Nov 8, 2024

Hey @olleolleolle, any chance of getting this merged? I’m also happy to address any suggestions if you have any.

Copy link
Collaborator

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked in detail at the individual code changes.

I think the fix is good, even with the early return, this method should not need other than the named branch, really. Any such changes are outside the scope of this repair.

Co-authored-by: Olle Jonsson <olle.jonsson@gmail.com>
@sdb9696
Copy link
Contributor Author

sdb9696 commented Nov 11, 2024

Thanks, I've applied the suggestions and retested the code and all works ok.

@sdb9696
Copy link
Contributor Author

sdb9696 commented Nov 18, 2024

Hey @olleolleolle, are you ok to merge this now?

@sdb9696
Copy link
Contributor Author

sdb9696 commented Nov 25, 2024

Hey @olleolleolle, sorry to pester but could we get this merged?

@olleolleolle olleolleolle merged commit 15516a6 into github-changelog-generator:master Nov 26, 2024
3 checks passed
@sdb9696 sdb9696 deleted the fix/missing_branches branch November 29, 2024 08:06
@sdb9696
Copy link
Contributor Author

sdb9696 commented Nov 29, 2024

Thanks for merging @olleolleolle. Do you plan to do a release anytime soon?

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