Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork849
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix missing release branch when looking for commits#1055
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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. |
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. |
The latest commit iterates the pages so no 100 limit either. |
Hey@olleolleolle, any chance of getting this merged? I’m also happy to address any suggestions if you have any. |
olleolleolle left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this 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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Olle Jonsson <olle.jonsson@gmail.com>
Thanks, I've applied the suggestions and retested the code and all works ok. |
Hey@olleolleolle, are you ok to merge this now? |
Hey@olleolleolle, sorry to pester but could we get this merged? |
15516a6 intogithub-changelog-generator:masterUh oh!
There was an error while loading.Please reload this page.
Thanks for merging@olleolleolle. Do you plan to do a release anytime soon? |
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.#971This 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.