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.

Sign up for GitHub

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

Jump to bottom

Adding a tip about a pull request's base branch #431

Closed
megbird opened this issue Oct 12, 2020 · 3 comments · May be fixed by venkatarao-palaparthi/docs#129, roru78/docs#4 or venkatarao-palaparthi/docs#178
Closed

Adding a tip about a pull request's base branch #431

megbird opened this issue Oct 12, 2020 · 3 comments · May be fixed by venkatarao-palaparthi/docs#129, roru78/docs#4 or venkatarao-palaparthi/docs#178
Labels
good first issue Good for newcomers

Comments

@megbird
Copy link
Contributor

megbird commented Oct 12, 2020

In these Halp tickets(content quoted 👇), users have asked us about what the expected behavior should be when a pull request's base branch is updated:

it seems like the .base.sha field is not always set to the actual base commit of the PR, but instead to the latest commit of the base branch at the time the PR got created.

Here's how to reproduce:

  1. Create a branch from master
  2. Add new commits to the branch
  3. Add new commits to master
  4. Create a PR for the branch
  5. Observe API results:

$ curl https://api.github.com/repos/neelance/apitest/pulls/1 | jq '.base.ref'
"master"
$ curl https://api.github.com/repos/neelance/apitest/pulls/1 | jq '.base.sha'
"c6e35b7668c32b21f08c8bad31e242f0862f6eb4"
$ curl https://api.github.com/repos/neelance/apitest/pulls/1/commits | jq '.[0].parents[0].sha'
"e726b8e3fb152fc4b172b234c69d884001fb87fa"

This behavior at least quite unexpected. I am currently also talking to the support of https://codecov.io/ because their system actually displays the wrong diff because of this.

also

I have a PR that has been opened for a few days now, and the base branch has been updated since then. However, while viewing diffs in the PR, it seems like the diffs are based on the base branch at the time of the creation of the PR.

So for example let's say my base branch is called "base" and the branch I want to merge is called "updates". Now on Monday I open a PR to merge updates into base. On Tuesday someone pushes new commits to base. The thing is, my PR opened on Monday doesn't take these in account, and the diff I'm viewing is the same I had on Monday.

Let's include a quick tip about it wherever best applicable, maybe in the Changing the base branch of a pull request article:

When you open a Pull Request, GitHub will set the base to the commit that branch references. If the branch is updated in the future, GitHub will not update the base branch's commit.
@megbird megbird added good first issue Good for newcomers triage Do not begin working on this issue until triaged by the team labels Oct 12, 2020
@janiceilene janiceilene removed the triage Do not begin working on this issue until triaged by the team label Oct 13, 2020
@Ruth-ikegah
Copy link
Contributor

I want to take this up!

@Ruth-ikegah
Copy link
Contributor

Can I use a block quote to indicate this tip?

@Ruth-ikegah Ruth-ikegah mentioned this issue Oct 13, 2020
Merged
4 tasks
@hubwriter
Copy link
Contributor

This has been done now. Closing

This was referenced Jan 11, 2022
Open
Open
@snyk-bot snyk-bot mentioned this issue Feb 8, 2022
Open
@JensDll JensDll mentioned this issue Mar 28, 2023
Closed
@izaitsevfb izaitsevfb mentioned this issue Apr 6, 2023
Closed
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this issue Apr 11, 2023
@izaitsevfb @pytorchmergebot
[stronghold][bc-linter] correctly determine the base commit of the PR (…
ec1d658 
…#98538)

Currently `${{ github.event.pull_request.base.sha }}` returns the HEAD of the base branch, which is different from **the base of the PR**.

See:
github/docs#431
https://github.com/orgs/community/discussions/39880

However, BC linter needs to know the base revision **of the PR**, as it looks at the changes **in the PR**.

This change is a workaround that determines the correct base of the PR. Hopefully in the future GH provides this information  in the event, and this workaround could be removed.
Pull Request resolved: #98538
Approved by: https://github.com/PaliC
@bstoll bstoll mentioned this issue Jun 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
4 participants
@megbird @janiceilene @hubwriter @Ruth-ikegah