"Old" PRs are not included in the release notes; seems like fromdate is not calculated correctly
Closed this issue · 10 comments
I've recently merged a few old PRs (months, not years), and it seems like they're not being found when creating a changelog. All of the "recent" PRs are included just fine.
My config is at: https://github.com/icosa-gallery/open-brush/blob/main/.github/workflows/build.yml#L419 (ignore the later invocation in the same file; that's not the problem).
This is a successful merge: https://github.com/icosa-gallery/open-brush/actions/runs/5668303523/job/15359591082 (look at the "Build Changelog" step). It reports:
ℹ️ Comparing icosa-gallery/open-brush - '2.3.36...9705caad484cb78e222b11b9b7ab1dc75705a0da'
ℹ️ Found 1 commits from the GitHub API for icosa-gallery/open-brush
ℹ️ Fetching PRs between dates 2023-07-26T11:42:25.000Z to 2023-07-26T11:42:25.000Z for icosa-gallery/open-brush
ℹ️ Retrieved 172 PRs for icosa-gallery/open-brush in date range from API
ℹ️ Retrieved 1 release commits for icosa-gallery/open-brush
ℹ️ Retrieved 1 merged PRs for icosa-gallery/open-brush
Compare that with the next build at https://github.com/icosa-gallery/open-brush/actions/runs/5679260667/job/15391975204
ℹ️ Comparing icosa-gallery/open-brush - '2.3.37...3b866a4ec779d4ba17397fb55dad4fbea3b21d67'
ℹ️ Found 1 commits from the GitHub API for icosa-gallery/open-brush
ℹ️ Fetching PRs between dates 2023-07-27T10:10:50.000Z to 2023-07-27T10:10:50.000Z for icosa-gallery/open-brush
ℹ️ Retrieved 87 PRs for icosa-gallery/open-brush in date range from API
ℹ️ Retrieved 1 release commits for icosa-gallery/open-brush
ℹ️ Retrieved 0 merged PRs for icosa-gallery/open-brush
In both cases, the dates look rather weird (the toDate is fine, but the fromDate looks wrong!), and it finds far fewer PRs to check, which, I'm assuming, is why it doesn't find the PRs that the second job came from.
Thank you very much for the detailed report.
Unfortunately GitHub does not offer a API to retrieve PRs within a given range of tags. As such the action has to go a different approach. E.g. it gets all commits between the range, and will then cross reference to PRs it fetches frmo the API.
That being said, yes, the from and to dates look off in that particular case, given they reference both the same date.
I will look into it.
(For reference, this are the default configs for the max back track time it usually uses: https://github.com/mikepenz/release-changelog-builder-action/blob/develop/src/configuration.ts#L64C35-L66)
Thanks! Yeah, the dates looked wonky to me, and also the different count of PRs seemed strange. If I can get you any other logs or anything else, please feel free to ask!
It's noteworthy that this line: Retrieved 172 PRs for icosa-gallery/open-brush in date range from API references to the raw - unmatched PRs. After this step we will get the has which the merge is associated with and cross reference them.
So in this case:
ℹ️ Retrieved 1 release commits for icosa-gallery/open-brush
ℹ️ Retrieved 1 merged PRs for icosa-gallery/open-brush
only 1 PR merge commit was cross references with a commit between the 2 tags.
I may not have time to look into it today, but I am positive that I'll have time tomorrow
Thanks a lot! No rush :-)
To be clear, one merged PR should be the correct answer for both jobs. This is the top of the repo:
3b866a4e (tag: 2.3.38) Snap improvements and Transform panel (#303)
9705caad (tag: 2.3.37) Update mikepenz/release-changelog-builder-action to v4 (#478)
19f4bde1 (tag: 2.3.36) Load HttpListener in a background thread; fixes delay on China mobile hotspots (#442)
6bfa2fe1 (tag: 2.3.35) Fix bug with snap rotation on rotated canvas. (#301)
98c78174 (tag: 2.3.34) Use GLTFast as the primary load; fall back to the old code if it fails (#278)
Each job ran on main, and it runs on each commit (and each commit gets tagged at the end of the release, which is why the from is a tag and the to is a commit).
Ok just had a closer look. So the problem is due to the way we need to cross reference PRs to commits.
Given in that case we retrieve the commits which have been in-between the tag to the hash:
Given you use squashed merges, there is only going to be the single (new commit) - with a creation date of the time of the merge.
However none of the commits from the original PR are there (thus the commit dates are not known) - https://github.com/icosa-gallery/open-brush/pull/303/commits
due to this the action won't attemp to fetch those old PRs (E.g. creation date October 31st)
That being said, I identified a differnet case here. As we would regardless get the first page of pull request. The problem here is that the sorting on merged on the API is not an option (anymore?) and as such the #303 PR is not included, even though it's the newest merged one.
Adjusting to sorting for updated seems to do the trick. Trying to think if there are implications to that which could cause other scenarios where PRs are missing.
Excellent, that looks great! I'll try this version once it's merged.
Would it also make sense for us to use max_back_track_time_days, or does this just just add filtering, rather than expanding the range? It wasn't clear to me from the docs if this can expand the window or only shrink it.
So I actually dove even deeper, and I identified a much better solution for scenarios where squashMerging is used.
GitHub has an API which allows to get PRs associated with a commit. While this is not what people want if they don't squash (as it's going to be quite heavy on the API rate limit -> 100 commits -> 100 requests) it seems to be perfect for squash commits where you have commits == PRs merged.
Should have something in the next hours :D (or to be more precise, I have it working - however I still need to add a new config for this mode)
Ok the new flag was merged into main and you can use it to enable this new behavior fetchViaCommits
(no release was made yet, there are some other changes pending I want to finalize first)
Awesome, I'll give the a shot over the weekend.