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

Implement file history walk in revwalk#889

Merged
johnhaley81 merged 6 commits intomasternodegit/nodegit:masterfrom
file-historynodegit/nodegit:file-historyCopy head branch name to clipboard
Feb 4, 2016
Merged

Implement file history walk in revwalk#889
johnhaley81 merged 6 commits intomasternodegit/nodegit:masterfrom
file-historynodegit/nodegit:file-historyCopy head branch name to clipboard

Conversation

@implausible
Copy link
Member

Implementing file history walk for revwalk. You pass it a sha, it then traverses until it either finds the 'added' entry for the file OR it hits the max number of commits to search for history.

@johnhaley81
Copy link
Collaborator

@implausible implausible changed the title [WIP] Implement file history walk in revwalk Implement file history walk in revwalk Feb 3, 2016
@implausible
Copy link
Member Author

@tbranyen @johnhaley81 once we get this to pass without some random test timing out, this is ready to go by my account. If you guys wanted to give this the once over before I merge in, that would be greatly appreciated.

Notes of interest: The fileHistoryWalk will do a rename detection if it discovers a file was added in the history and will return the old file name and the renamed status along with the commit. This helps history users to not only follow the filepath's history, but also see how the content if the file has changed / branched.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see these getting freed anywhere. Is this memory cleaned up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find whether or not we're supposed to clean it up or not. It would be super helpful if the docs gave me some insight

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you know explicitly that we're supposed to clean those up, I'll free the. I've been mulling it over myself, because the docs are not clear!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh cool! I guess since it's explained there they don't have to explain it in the function documentation for tree to tree.

@johnhaley81
Copy link
Collaborator

Other then the potential memory leak above, could you get some comment blocks into lib/revwalk/js for docs?

Only find similar if there is an added entry
Free diffs when we are done with them.
}
}

git_diff_free(diffs);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Freeing diffs down here now.

@implausible
Copy link
Member Author

@johnhaley81 @tbranyen what is going on with this test? https://travis-ci.org/nodegit/nodegit/jobs/106996362

johnhaley81 added a commit that referenced this pull request Feb 4, 2016
Implement file history walk in revwalk
@johnhaley81 johnhaley81 merged commit 2fca86e into master Feb 4, 2016
@johnhaley81 johnhaley81 deleted the file-history branch February 4, 2016 20:23
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.

2 participants

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