Go-github: Question about update branch API

Created on 29 Jul 2019  路  7Comments  路  Source: google/go-github

I'm using this library for development on GitHub bots. Thanks for this awesome library!

While trying UpdateBranch service, I found that this API will do update branch all the time, even if the base branch is up to date. An empty udpate branch action will make a new head sha of the pull request. Is there some methods to check if the branch is up to date for avoiding empty update branchs?

Most helpful comment

After running this update branch comparation for 2 days, I found that compare SHA of PR's base branch and SHA of the latest commit to target branch only works when commits to target branch are created after PR is created.
Consider for this situation, checkout from master in 2 pm, working for 2 hours and create a PR with master as target branch in 4 pm, however, there is a commit to master in 3 pm. The base SHA is same as latest commit SHA in master, but an update branch action is required.

C7134546641A62B90749881BA49AE816

We should inspect into the commit tree, and I made some updates on the snippet before.

pr, _, _ := client.PullRequests.Get(context.Background(), owner, repo, id)
baseCommits, _, _ := client.Repositories.ListCommits(context.Background(), owner, repo, &github.CommitsListOptions{
    SHA: pr.Base.GetRef(),
})
headCommits, _, _ := client.Repositories.ListCommits(context.Background(), owner, repo, &github.CommitsListOptions{
    SHA: pr.Head.GetSHA(),
})

needUpdateBranch := true
baseSHA := baseCommits[0].GetSHA()
for _, commit := range headCommits {
    if baseSHA != "" && commit.GetSHA() == baseSHA {
        needUpdateBranch = false
    }
}

if needUpdateBranch {
    client.PullRequests.UpdateBranch(context.Background(), owner, repo, *pr.Number, nil)
}

Although if the lastest base branch commit SHA is not in first page of head branch tree will create an empty "update branch", it will not do harm to anyone.

All 7 comments

I'm wondering if you need to check the most recent commit hash to see if it is newer than the last branch update prior to updating? If so, you might need to involve the use of git.

This might be a good question for [email protected] unless someone else has some good ideas.

commits, _, _ := client.Repositories.ListCommits(context.Background(), owner, repo, nil)
pr, _, _ := client.PullRequests.Get(context.Background(), owner, repo, id)
if *commits[0].SHA != *pr.Base.SHA {
    client.PullRequests.UpdateBranch(context.Background(), owner, repo, *pr.Number, nil)
}

@gmlewis thanks, this code seems working for me. Problem resolved, close issue.

That's great, @you06! Thanks for reporting back.

If this were a code review, I might comment that you could potentially use the getters to prevent the possibility of a nil dereference, like so:

if commits[0].GetSHA() != pr.Base.GetSHA() {
...

I'm glad you found a working solution.

Thanks for your CR @gmlewis! I'm using get functions now, and here's an implementation for all branches not only master.

pr, _, _ := client.PullRequests.Get(context.Background(), owner, repo, id)
commits, _, _ := client.Repositories.ListCommits(context.Background(), owner, repo, &github.CommitsListOptions{
    SHA: pr.Base.GetRef(),
})
if commits[0].GetSHA() != pr.Base.GetSHA() {
    client.PullRequests.UpdateBranch(context.Background(), owner, repo, *pr.Number, nil)
}

Looks great. But if I'm really going to do a code review, I would say "Don't ignore the returned errors." :smiley:

It just takes a couple times for this to bite you big time in production before you decide to never again ignore the returned errors. :smile:

Sure, this snippet is just for a demo, so I make it in the easiest way.

After running this update branch comparation for 2 days, I found that compare SHA of PR's base branch and SHA of the latest commit to target branch only works when commits to target branch are created after PR is created.
Consider for this situation, checkout from master in 2 pm, working for 2 hours and create a PR with master as target branch in 4 pm, however, there is a commit to master in 3 pm. The base SHA is same as latest commit SHA in master, but an update branch action is required.

C7134546641A62B90749881BA49AE816

We should inspect into the commit tree, and I made some updates on the snippet before.

pr, _, _ := client.PullRequests.Get(context.Background(), owner, repo, id)
baseCommits, _, _ := client.Repositories.ListCommits(context.Background(), owner, repo, &github.CommitsListOptions{
    SHA: pr.Base.GetRef(),
})
headCommits, _, _ := client.Repositories.ListCommits(context.Background(), owner, repo, &github.CommitsListOptions{
    SHA: pr.Head.GetSHA(),
})

needUpdateBranch := true
baseSHA := baseCommits[0].GetSHA()
for _, commit := range headCommits {
    if baseSHA != "" && commit.GetSHA() == baseSHA {
        needUpdateBranch = false
    }
}

if needUpdateBranch {
    client.PullRequests.UpdateBranch(context.Background(), owner, repo, *pr.Number, nil)
}

Although if the lastest base branch commit SHA is not in first page of head branch tree will create an empty "update branch", it will not do harm to anyone.

Was this page helpful?
0 / 5 - 0 ratings