Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show base commit in PRs (and not just the base branch) #3863

Closed
fregante opened this issue Jan 6, 2021 · 7 comments · Fixed by #6538
Closed

Show base commit in PRs (and not just the base branch) #3863

fregante opened this issue Jan 6, 2021 · 7 comments · Fixed by #6538
Labels
enhancement help wanted Please! ♥︎ Particularly useful features that everyone would love!

Comments

@fregante
Copy link
Member

fregante commented Jan 6, 2021

PRs show the base branch, but they don't tell how long ago the current branch was updated. Example:

master         A-B-C-D-E  
                    \     
feature-branch       C-Q-Z    

In this case, this feature would tell us that C is the real base

I've wanted to know this in the past, but at the moment I'm only thinking of why we shouldn't implement this: 😅

  • Our update-pr-from-base-branch feature already tells us whether it's up to date and it allows us to update it

Discussion welcome. Would this info be useful? When?

@fregante
Copy link
Member Author

fregante commented Jun 5, 2022

This would use the same API call needed for

@cheap-glitch
Copy link
Member

This would use the same API call

Maybe there's a shorter way but this works pretty well:

{
  repository(<...>) {
    pullRequest(number: <number>) {
      commits(first: 1) {
        nodes {
          commit {
            parents(first: 1) {
              nodes { oid }
            }
          }
        }
      }
    }
  }
}

Will try to open a PR today if I can find the time.

@fregante
Copy link
Member Author

@cheap-glitch That doesn't work because it always uses the first commit’s parent, but actually the base can change with a merge commit:

master         A-B-C-D-E-F  
                    \     \
feature-branch       C-Q-Z-Y    

Your query would return C, but GitHub compares the files with F. Our current baseRefOid already updates correctly, except when there's a force push.

For this feature we can use this value for now. My question is just: where do we show this piece of information? restore-file and update-pr-from-base-branch should already have queried all the necessary information here.

@fregante
Copy link
Member Author

where do we show this piece of information?

What if we turn update-pr-from-base-branch into a panel like:

HEAD is 3 commits behind BASE (abcdefg). [Update branch]

This would not appear when the PR is up to date and that's ok.

We'd show the banner regardless of conflicts and native "update branch" buttons. We can preserve this logic just to determine whether to show our Update button:

@yakov116
Copy link
Member

@fregante is this the style you had in mind?

image

		if (pageDetect.isEnterprise()) {
			position.append(' ', (
				<span className="status-meta d-inline-block rgh-update-pr-from-base-branch">
					You can <button type="button" className="btn-link">update the base branch</button>.
				</span>
			));
		} else {
			position.append(' ', (
				<span className="status-meta d-inline-block">
					{select('.head-ref')!.cloneNode(true)} is {prInfo.headRef.compare.behindBy} commits behind {select('.base-ref')!.cloneNode(true)}
					 {' '}(<a className="btn-link" href={buildRepoURL('commits/' + prInfo.baseRefOid)}>{prInfo.baseRefOid.slice(0, 8)}</a>)
					[<button type="button" className="btn-link rgh-update-pr-from-base-branch">update branch</button>].
				</span>
			));

@fregante
Copy link
Member Author

Love it. I didn't know where to display it, but that makes sense. We can figure out the exact design later

@yakov116
Copy link
Member

OK we need to merge #6103 first and then I can work on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted Please! ♥︎ Particularly useful features that everyone would love!
3 participants