Conversation
| </a> | ||
| ); | ||
| return ( | ||
| <>This branch is {countLink} behind the base branch (base commit: {commit})</> |
There was a problem hiding this comment.
Ideas? This kinda repeats the title, we can shorten it further
There was a problem hiding this comment.
This could work:
Ideas on how to integrate the "base commit" info? It's not a super clear concept 🤔
There was a problem hiding this comment.
The drawback of this position is that the DOM may vary a lot, which is actually why we pulled update-pr-from-base-branch out of the mergeability box years ago.
It's easier for us to handle it now, but there's still some UI variability that makes this a little complex 🥲 (e.g. PR with conflicts don't show pr-base-commit right now)
source/github-helpers/get-pr-info.ts
Outdated
| const {pullRequest} = repository; | ||
| return { | ||
| ...repository.pullRequest, | ||
| behindBy: compare.behind_by, |
There was a problem hiding this comment.
Interestingly the count seems wrong, there's currently just one commit.
Base commit: https://github.com/refined-github/sandbox/commits/2723dc763d512e421d2562c1da04b6cb14ba8a12
Base branch: https://github.com/refined-github/sandbox/commits/default-a
Comparison: refined-github/sandbox@2723dc7...default-a
You can see commit 2723 (base commit) is the second one and only 59a2 is missing.
There was a problem hiding this comment.
Same thing on refined-github/sandbox#61 except it says "1 commit" even if there are actually 3 😳
There was a problem hiding this comment.
I remember having this issue somewhere but I can't remember where.
Crazy idea but I though of using
#6124 (comment)
Then parsing the dom
Also do v3 and v4 return the same number?
There was a problem hiding this comment.
Maybe by the ahead count since the last release? (Sorry on mobile and do not remember the feature name)
There was a problem hiding this comment.
Found the reason: the comparison is flipped, the API returns "main is behind pr-branch", which is not how we're been interpreting it.
This comparison: refined-github/sandbox@bbbe3aa...default-a
Before
After (using prInfo.behindBy = comparison.aheadBy)
fregante
left a comment
fregante
left a comment
There was a problem hiding this comment.
Test results
PR
Base branch
https://github.com/refined-github/sandbox/commits/default-a
Head branch
https://github.com/refined-github/sandbox/commits/6538-PR-BRANCH
Result
- it's 1 commit behind ✅
- the base commit is 0dde998 ✅
- the
1 commitlink points to refined-github/sandbox@0dde998...default-a ✅ - the compare link shows
1 committoo ✅ - the compare link shows only the missing commit ✅
Base commit
- Hovercard works ✅
- Commit link points to single commit ✅ refined-github/sandbox@0dde998
| <p><a title="reactions-avatars"></a> Adds reaction avatars showing <i>who</i> reacted to a comment | ||
| <p><img src="https://user-images.githubusercontent.com/1402241/130341871-6a0d69f4-8d0c-4882-a5ed-aac9b7613b0a.png"> | ||
| <p><a title="pr-base-commit"></a> Shows how far behind a PR head branch is + tells you its base commit | ||
| <p><img src="https://user-images.githubusercontent.com/1402241/234492651-b54bf9ba-c218-4a30-bed4-f85a7f037297.png"> |
There was a problem hiding this comment.
Final look:
reaction-avatars downgraded
| } | ||
| `); | ||
|
|
||
| const compare = await api.v3(`compare/${base}...${head}?page=10000`); // `page=10000` avoids fetching any commit information, which is heavy |
There was a problem hiding this comment.
It's been 6 months. Dropping
fregante
left a comment
fregante
left a comment
There was a problem hiding this comment.
Also tested with merges.
PR
Step 1: See PR that needs update ✅
Step 2: "Update branch" ⚠️
update-pr-from-base-branch caused the mergeability widget to not be updated due to an internal warning (Failed to update content with interactions
Screen.Recording.mov
Step 3: refresh ✅
Refreshing doesn't show pr-base-commit because it's up to date
Step 4: commit on main ⚠️
- The UI correctly detects it's 1 commit behind ✅
- The
1 commitpage shows exactly 1 commit ✅ - The base commit is "wrong" (it's the same as before) ❌
|
Ok. This conversation/review/testing is long enough. Merging even though the "base commit" isn't what we expect at the moment. We can consider hiding it via hotfix if it's a problem and then go back to the drawing board. |
2 participants
update-pr-from-base-branch#6537PR without conflicts
refined-github/sandbox#60
Draft PR without conflicts⚠️
update-pr-from-base-branchis also enabled, because that's what creates the whole "green" row in the following screenshotrefined-github/sandbox#61
Native "Update branch" button
#6531
(or pick a conflict-free PR from https://github.com/refined-github/refined-github/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc)
Native "Resolve conflicts" button ❌
refined-github/sandbox#9