Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit.getParents working with merge commits#357

Merged
johnhaley81 merged 1 commit intonodegit:masternodegit/nodegit:masterfrom
bjornarg:get-multiple-parentsbjornarg/nodegit:get-multiple-parentsCopy head branch name to clipboard
Jan 19, 2015
Merged

Commit.getParents working with merge commits#357
johnhaley81 merged 1 commit intonodegit:masternodegit/nodegit:masterfrom
bjornarg:get-multiple-parentsbjornarg/nodegit:get-multiple-parentsCopy head branch name to clipboard

Conversation

@bjornarg
Copy link
Contributor

getParents failed when more than one commit was present, since it attempted
to get the second parent of commit's parent instead of it's own second
parent.

The promise for the second parent was also pushed onto the array for
Promise.all() too late to actually have an effect.

Limit was not working properly as a limit, since it attempted to
retrieve the set limit regardless of how many parents a commit had,
failing if the limit was set to more than Commit.parentcount().

@tbranyen
Copy link
Member

Awesome thank you @bjornarg!

@johnhaley81
Copy link
Collaborator

Re-queued this to test on AppVeyor. If it passes I'll go ahead and merge it in.

@johnhaley81
Copy link
Collaborator

Ok so it didn't try to do a merge with the latest on master. @bjornarg could you rebase this commit on the latest in master please and then we can go ahead and merge after tests pass.

Thanks!

getParents failed when more than one commit was present, since it attempted
to get the second parent of commit's parent instead of it's own second
parent.

The promise for the second parent was also pushed onto the array for
Promise.all() too late to actually have an effect.

Limit was not working properly as a limit, since it attempted to
retrieve the set limit regardless of how many parents a commit had,
failing if the limit was set to more than Commit.parentcount().
@bjornarg
Copy link
Contributor Author

@johnhaley81 You want me to force-push a rebase?

@bjornarg bjornarg force-pushed the get-multiple-parents branch from 30ab7c8 to 00a62f8 Compare January 19, 2015 20:48
@bjornarg
Copy link
Contributor Author

Well, seems like travis is having trouble now... :)

@maxkorp
Copy link
Collaborator

maxkorp commented Jan 19, 2015

Yeah go ahead and rebase and force push, and we'll bring it in upon test pass. We've been having a few hiccups with builds just randomly stalling today, but it all seems resolved (knock on wood).

@johnhaley81
Copy link
Collaborator

All tests passed, thanks for the PR @bjornarg!

johnhaley81 added a commit that referenced this pull request Jan 19, 2015
Commit.getParents working with merge commits
@johnhaley81 johnhaley81 merged commit ad21b7a into nodegit:master Jan 19, 2015
@maxkorp
Copy link
Collaborator

maxkorp commented Jan 19, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.