mikepenz / release-changelog-builder-action

A GitHub action that builds your release notes / changelog fast, easy and exactly the way you want.
https://blog.mikepenz.dev
Apache License 2.0
684 stars 99 forks source link

Missing PRs randomly in release notes #1300

Closed michaelrosett closed 5 months ago

michaelrosett commented 5 months ago

After hours of local debugging, it seems that this query (code below) doesn't produce a complete list of PRs for me. I don't believe sort: 'merged' is a supported sort order. When I change this to sort: 'updated, then all (at least more) my of PRs show up. Initially, my fromTag and toTag yielded 294 commits and 24. After after this change, it yields 294 commits and 267 PRs.

const options = this.octokit.pulls.list.endpoint.merge({
      owner,
      repo,
      state: 'closed',
      sort: 'merged',
      per_page: `${Math.min(100, maxPullRequests)}`,
      direction: 'desc'
    })

Would it be possible to fix and create a new release for me? We've been seeing this odd behavior for over a year now and my coworkers keep asking why the release notes are missing their PRs and that QA doesn't have a complete list of PRs to test. I can send you a PR if that moves things along. Not sure if there's an appropriate test for this.

michaelrosett commented 5 months ago

According to https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests, sort only supports the values created, updated, popularity, long-running

mikepenz commented 5 months ago

Thank you very much for raising this ticket.

In the past merged did work, and we have a good amount of tests to check if PRs are retrieved. That said I wonder if because it is not on the docs (anymore(?)) it can have undefined behaviour.

That said, there is a big problem going with updated as it will return many more items (basically any PR you make any change to, even if it's just updating the description) will be sorted ahead.

There are certainly additional implications we need to be careful about, for example fetchedEnough would very likely need to also be adjusted to only consider the update date.

Is there any way you can reproduce this consistently on a open source repo? That would immensely help to setup a test case for it.

mikepenz commented 5 months ago

In the process of tagging a release candidate v4.2.0-rc01 which you will be able to use

michaelrosett commented 4 months ago

It would be difficult to reproduce this as it has to do with the number of PRs and the blackbox that is the GitHub API - it is unclear what underlying factors cause unpredictable response ordering.

I have verified the new release candidate against tags in our private repo where we saw the issue. Thank you!

mikepenz commented 4 months ago

Thank you very much for verifying the changes!