PR help



  • I am having a terrible time getting my Pull Requests to work the way i think they should. Mainly because some of the time they do work the way i think they should.

    My problem is that i am constantly getting old commits mixed in with my new PRs. They part that is really confusing me is that I branched my local master, made a commit with that branch yesterday, and it only had the one commit. I then branched it again today, made another commit, but this time it has commits in the PR from 3-4 weeks ago as well.

    I am using Intellij, and not using the command line.
    Thanks!



  • Can you point to the PRs?

    Are these old commits things you did or are they from other people?



  • This one has extra commits. I closed it before it got merged in, and eventually got it to work without the extra commits.
    https://github.com/triplea-game/triplea/pull/8114

    Here is where i got it to work. https://github.com/triplea-game/triplea/pull/8120

    They are old commits to my origin/master branch. some of them are when i merged the upstream/master into it, some are just previous commits that I did for PRs to the upstream/master.


  • Admin

    @ff03k64 are you using command-line or are you using a GUI for git?

    If you are using CLI, and you have a remote named upstream, this is roughly the command:

    git fetch upstream
    git pull --rebase origin master
    

    One thing to look at is if your branch is 'behind' or ahead of master. This is the number of commits you have (or do not have) relative to a base branch.

    In the branches view of your repo:
    Screenshot from 2020-11-09 17-57-55.png

    You can note that a few of those branches are just one commit ahead. When you PR, you'll have just the one commit.

    One thing to keep in mind is that there is nothing special about any branch relative to another.

    A pull request is a request to some other maintainer to pull your branch into theirs. So when you are behind, you do a pull --rebase to pull those changes in. When you are doing a PR, it is the other person that is in effect doing a pull --rebase to pull your changes into their branch.

    So, it's all about seeing where branches are and which commits they have. If you rebase one branch onto another, then the commits from the one branch are added on top of the other.

    For example, in the above there are a few branches that are quite behind, but have a few commits ahead. If you rebase those, then they will be just ahead and will not be behind anymore as all of the new commits from the remote branch will be pulled in.

    Hope this helps, let us know if you keep getting stuck.



  • @ff03k64 , it looks like you are merging the triplea master into your master and then merging other things into it. Here's your master commits: https://github.com/CharlieAtlas/triplea/commits/master.

    I can see a "Merge remote-tracking branch 'origin/master'" commit and a "Merge pull request #5 from triplea-game/master" commit.

    Your master should be identical to the master in triplea-game. You should never commit or merge anything into your master. You just need to rebase the commits.

    In my git repo, I have two remotes: github git@github.com:trevan/triplea.git and upstream https://github.com/triplea-game/triplea.git. You should also have two remotes. In IntelliJ, you can see them by going to VCS -> Git -> Remotes...

    When you are ready to make a branch, you should fetch the latest data (VCS -> Git -> Fetch) and then branch off of the master of triplea-game. To do that, go to VCS -> Git -> Branches and then filter by master. There should be a "Local" master and two "Remote" masters. You'll want to pick the one that is from the triplea remote (in my case, it is the upstream remote). That will checkout the latest from triplea and then you can create a new branch.



  • @ff03k64 , you'll want to fix up your master in your github account. If you don't have any branches outstanding, the easiest way would be to delete the fork and refork it. Otherwise, you'll need to do force pushes to change the HEAD of the master.



  • @LaFayette
    I am using the gui in IntelliJ.

    For those branches that are one commit ahead (in theory) are the one commit ahead of my origin master? Or the upstream master? I am guessing the upstream because if i do a PR, that is where they go.

    The readme and PRmd branches are currently showing extra commits when i go to make a Pull Request. Can you see that? Or just my branches?

    Let me describe some more. Yesterday I did a PR that you just accepted this evening. I branched off my local master for it. I made 2 new branches off my local master this morning, PRmd and readme. They both have the extra commits in then from over the course of the last month.

    the only thing i can think of is that i rebased my local master off my origin master?


  • Admin

    @ff03k64 said in PR help:

    For those branches that are one commit ahead (in theory) are the one commit ahead of my origin master? Or the upstream master? I am guessing the upstream because if i do a PR, that is where they go.

    Ahead of your master. One thing to keep in mind is that Git is a distributed VCS. Every branch, every repo, is it's own authoritative source. It is built so that once you fork, you have your own fully fledged copy that becomes independent of any other code base.

    Trevan described the flow well:

    • start with your master that is even with upstream. You can compare the last commit of what you have vs upstream by looking at commits and comparing the commit SHA1 values.
    • fetch the latest and rebase to the lastest upstream
    • create a new branch from there
    • commit stuff
    • push that branch
    • PR that branch
    • delete that branch.

    When your branch is merged into the upstream master, you'll pick up the changes when you rebase to the latest upstream.

    Re: fixing up your branches

    When you branch, no commits are removed, you simply start a new branch. If you have stuff merged to master, then create a new branch, you'll still have that 'stuff'. Your master being ahead of upstream brings this along, and any branch created from there will PR that same stuff as it's a commit that is ahead of upstream master.

    There is a slightly easier way to fix your master:

    checkout master from upstream

    git branch -D master
    git fetch upstream
    git checkout -b master upstream/master
    git push -f origin master
    

    remove the commits via history rewrite

    git fetch upstream
    git checkout master
    git reset --soft HEAD~n  # where n = number of commits ahead + 1
    git checkout .
    git status
    # 'rm' any files shown as added but not tracked
    git pull --rebase upstream/master
    


  • @LaFayette said in PR help:

    When you branch, no commits are removed, you simply start a new branch. If you have stuff merged to master, then create a new branch, you'll still have that 'stuff'. Your master being ahead of upstream brings this along, and any branch created from there will PR that same stuff as it's a commit that is ahead of upstream master.

    I understand that no commits being removed on branch, but i am pretty sure i had a time where i branched, did a PR, and it worked correctly. Next day, did another branch and the PR from that branch had old commits, even though the one before that didn't have any old commits.
    I didn't change anything in between the two PR creations. But it still had old commits on the later one. The branches were created off of a local master.
    That is what is really confusing me. The local master should have none of those old commits at that point associated with it. Right?


  • Admin

    Here is a long example, hopefully it helps 🤞

    @ff03k64

    Let's see if this can be clarified with an example. There are a number of branches in play and there are three repositories. Repositories:

    • upstream github triplea-game/triplea
    • forked github triplea-game/triplea
    • local cloned 'triplea' (on your machine)

    On all of these we have a master branch. Those master branches are physically different, but by convention they are made to 'track' one another and are kept in-sync. Let's give these branches names:

    • UM = upstream master (in github)
    • FM = forked master (in github)
    • LM = local master (on your machine)

    Now, let's say we are going to create so-called feature branches which we will use for PR. For an example, let's have 3 of these. Which we will call:

    • FFB1 = forked feature branch 1 (lives in github)
    • LFB1 = local feature branch 1 (lives on your local machine)
    • FFB2 = forked feature branch 2 (lives in github)
    • LFB2 = local feature branch 2 (lives on your local machine)

    Scenario 1 - single feature branch flow

    Okay, let's say we are just starting out and UM has two commits, {a, b}. With fresh forks and clones, everything is all even and so we have this state:
    UM = {a, b}
    FM = {a, b}
    LM = {a, b}

    By default, you have 'master' checked out and you create a feature branch locally. Now we have a new feature branch locally:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b}

    Now you do work in LFB1 and commit, commit 'c', we now have:
    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}

    Now you push LFB1 to your fork, we now have a new branch in the forked repository.

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}

    Now you do a PR of FFB1 into UM, and let's say it is merged. We do squash rebase which changes the merged commits. Let's call the squashed rebased commit 'c' to be 'd'.

    After merge of LFB1 into UM, we have:
    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}

    After merge, feature branches should be deleted from both your local repository and fork. Cleaning up, we have:

    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b}

    Now we want to update our LM to have the latest commits. We checkout 'master' locally (LM), and do a pull --rebase to get the latest, we now have:

    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b, d}

    Now we update our forked master by pushing LM to the fork, which changes "FM" to be current:

    UM = {a, b, d}
    FM = {a, b, d}
    LM = {a, b, d}

    Now we can repeat the above process with feature branches.

    Scenario 2 (bad) - feature branch from feature branches

    One think to avoid is creating feature branches from other feature branches.

    Let's go back to the scenario where we have a fresh fork, fresh clone, and we create a fresh feature branch:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b}

    Now we commit onto LFB1, commit 'c'. We get:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}

    Now we create a new feature branch from LFB1 (instead of local master, this is a minor mistake), we get:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    LFB2 = {a, b, c}

    Now we do work in LFB2 and commit, commit 'e', We get:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    LFB2 = {a, b, c, e}

    Now we push up both feature branches to create two new branches in the forked repository. We get:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}
    LFB2 = {a, b, c, e}
    FFB2 = {a, b, c, e}

    Now we PR FFB1 into UM and let's say it is merged, and commit 'c' becomes commit 'd'. We get:

    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}
    LFB2 = {a, b, c, e}
    FFB2 = {a, b, c, e}

    Because the first branch was merged, we clean it up and delete it from local repository and the forked repository, now we have:

    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b}
    LFB2 = {a, b, c, e}
    FFB2 = {a, b, c, e}

    Now, let's say we create a PR of FFB2 into UM. The common root of the two branches is {a, b}. That means FFB2 is two ahead and one behind. The PR would seek to add the new commits onto UM, so the PR will show as adding commits "c, e". Now, "c" and "d" have the same content, the same commit message, but if you look closely their commit SHA (their ID) will be the only difference. So in the PR commit 'c' will appear as an empty commit. When you rebase, empty commits drop away.

    So if you rebase LFB2 onto UM, first git takes the remote branch:

    {a, b, d}

    Then git will compute the common ancestory, in this case {a, b}

    Then git will add, one by one, each commit from the rebasing branch (LFB2) and you'll know have the remote branch: {a, b, d} with new incoming commits {c, e}. That gives {a, b, d, c, e}. 'c' is an empty commit, the result will then be: {a, b, d, e}

    Now if you PR the branch {a, b, d, e}, into UM (which has {a, b, d}), the incoming branch is exactly one ahead and you'll see just the new commit 'e'.

    Scenario 3 - multiple feature branches

    Okay, to wrap it up, let's say we create multiple feature branches. Let's start with the initial state:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}

    We create one feature branch, add a commit 'c' and push it:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}

    Now we checkout our local master and create a new faeture branch from that:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}
    LFB2 = {a, b}

    Now we do some work in LFB2 and commit, commit 'z':

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}
    LFB2 = {a, b, z}

    Now we push LFB2 to our fork:

    UM = {a, b}
    FM = {a, b}
    LM = {a, b}
    LFB1 = {a, b, c}
    FFB1 = {a, b, c}
    LFB2 = {a, b, z}
    FFB2 = {a, b, z}

    Now we PR FFB1 into UM and mege, commit 'c' becomes commit 'd', and we clean up the first feature branches leaving:

    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b}
    LFB2 = {a, b, z}
    FFB2 = {a, b, z}

    Now, if we PR FFB2 into UM, the common ancestor is {a, b} and it'll be a PR to add commit 'z'. The fact that FFB2 does not have 'd' just means FFB2 is behind. It's best practice to never PR a branch that is behind, instead update it to the latest first so you know you have all of the latest code and are testing with the very latest.

    So, let's say we rebase LFB2 onto UM (ie: we checkout LFB2 locally, then run git pull --rebase upstream master).

    Git, locally, will take the branch we are rebasing onto as the start: {a, b, d}. Git will then compute the common ancestor of the two branch: {a, b}, it will then add, one by one all new commits on the rebasing branch onto the rebase target. In this case we have branch {a, b, z} rebasing onto {a, b, d} the common ancestor is {a, b}, that means git will add commit 'z' onto {a, b, d} resulting in {a, b, d, z}. Now we have:

    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b}
    LFB2 = {a, b, z}
    FFB2 = {a, b, d, z}

    Now we can no longer push FFB2 up to our fork without issue. Notice we would be inserting commit 'd', that is not allowed. You can only add commits with a push, never insert them. To fix this we have to 'force' push which will take our local branch and overwrite what we have remotely. So if we force push 'LFB2' to our fork, we then get:

    UM = {a, b, d}
    FM = {a, b}
    LM = {a, b}
    LFB2 = {a, b, d, z}
    FFB2 = {a, b, d, z}

    Now if we PR FFB2 into UM, we are PR'ing just the one commit 'z'. Let's say that is merged and commit 'z' becomes commit 'x'. We get:

    UM = {a, b, d, x}
    FM = {a, b}
    LM = {a, b}
    LFB2 = {a, b, d, z}
    FFB2 = {a, b, d, z}

    Now FB2 is merged, so we delete the two branches for it, the local and fork, which gives:

    UM = {a, b, d, x}
    FM = {a, b}
    LM = {a, b}

    Now notice that LM and FM are behind, which means if we create feature branches from LM it will start out behind. To fix this we rebase LM onto UM, which brings it up to date:

    UM = {a, b, d, x}
    FM = {a, b}
    LM = {a, b, d, x}

    Now we can push 'LM' to our fork, becuase we are only adding commits we can do a regular push, which then leaves:

    UM = {a, b, d, x}
    FM = {a, b, d, x}
    LM = {a, b, d, x}



  • @LaFayette said in PR help:

    Now we create a new feature branch from LFB1 (instead of local master, this is a minor mistake), we get:

    You said scenario 2 was bad, is this why? Or just poor practice and something else in there was bad?

    I think scenario 3 has basically been what i have been doing. In your terms, here is my process all withing 48 hours.

    UM, FM, and LM are all made
    LFB1 branched from LM
    LFB1 commits made and pushed to FFB1
    made a PR for FFB1
    LFB2 forked from LM (it might have been a couple behind UM at this point)
    LFB2 commits made and pushed to FFB2
    made a PR for FFB2, and it had commits from a couple weeks prior that I can't figure out how they are getting into the PR.

    I was able to rebase UM onto LM after that, and recommit. At which point i got FFB2 without the extra commits. I could see there being extra commits because it is behind, but why are there extra commits from prior to my rebasing it?

    So by the sound of it, I think I have the basic idea of what to do correct, there is just something weird going on that I would like to figure out.


  • Admin

    @ff03k64 sounds like you have the picture. Checking history with 'git log' can help to compare commit sha's. Some git versions will indicate which branches are at which commit in that same view.



  • @LaFayette I am beginning to think there are different ways to rebase. I did it twice today on different branches using the GUI (VCS->Git-> pull -rebase upstream), and one didn't have extra commits and the other did. Then i rebased it a different way (VCS -> rebase my github fork -> upstream) and i the second one didn't have the extra commits anymore.

    Any idea what the difference between them is? Or did it just not like me the first time?


  • Admin

    There are three ways to merge branches:

    1. merge
    2. fast forward merge
    3. rebase

    git merge <remote> <branch> and git pull <remote>/<branch> AFAIK are the same thing and are both merges.

    git pull --rebase <remote> <branch> is a proper rebase. This is the one you should prefer to avoid having 'merge commits'.

    A fast forward merge is really similar to a rebase, and you do that with a -ff flag to merge, ie: git merge -ff <remote> <branch>

    There is also a proper 'rebase' that can be done. AFAIK these two are the same:
    git pull --rebase <remote> <branch> and git rebase <remote> <branch>

    Unfortunately it is fundamentally a bit confusing as it depends on exactly how the rebase is done and with which flags. This is a place where a GUI can get in the way.


Log in to reply