fix: PR lookup for PRs with no backports #34
Conversation
codebytere
force-pushed
the
fix-data-fetching
branch
from
9cba3e6 to
a1a0983
Compare
| @@ -1,7 +1,6 @@ | |||
| const Handlebars = require('handlebars'); | |||
| const { Router } = require('express'); | |||
| const semver = require('semver'); | |||
| const MarkdownIt = require('markdown-it'); | |||
There was a problem hiding this comment.
Left from previous PR
| @@ -132,7 +128,6 @@ async function getPRReleaseStatus(prNumber) { | |||
|
|
|||
| // This is the primary PR, we can scan from here for backports | |||
| return { | |||
| highlightPR: prNumber, | |||
There was a problem hiding this comment.
Beyond the stated fix, this PR also makes data fetching more consistent and adds error logging in event of failure. This caused an issue for me while i was debugging as I'd not properly authenticated with octokit and was getting rate limited without realizing it.
I'm not a fan of wrapping these in try/catch with empty results since they're memoized and the empty results will be cached. Especially for getReleasesOrUpdate, I wouldn't want a transient error causing an empty response which is then cached. Users hitting those endpoints should get some kind of error so that they know the response is not valid.
Can we find a middle ground with extra logging but still throwing the errors there?
|
@dsanders11 fair enough - i can at minimum split it into another PR |
codebytere
force-pushed
the
fix-data-fetching
branch
3 times, most recently
from
4c5fd05 to
2133501
Compare
codebytere
force-pushed
the
fix-data-fetching
branch
from
2133501 to
85a8a17
Compare
This PR fixes an issue where the user would incorrectly be redirected to the homepage when trying to look up certain PRs. This was happening because of an incorrect assumption being made that the base ref would be main if it was merged to the default branch, which wouldn't be true for older branches merged to
masterbefore the rename.