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

Let my patches go#930

Merged
johnhaley81 merged 2 commits intomasternodegit/nodegit:masterfrom
let-my-patches-gonodegit/nodegit:let-my-patches-goCopy head branch name to clipboard
Feb 29, 2016
Merged

Let my patches go#930
johnhaley81 merged 2 commits intomasternodegit/nodegit:masterfrom
let-my-patches-gonodegit/nodegit:let-my-patches-goCopy head branch name to clipboard

Conversation

@joshaber
Copy link
Collaborator

Fixes #877.

It looks like #883 was supposed to fix this, but we missed the part where we actually did the free’ing. 🙈

This is only WIP because I’m still trying to make a build so I can test that this fixes the whole problem.

@joshaber
Copy link
Collaborator Author

(Also cc atom/atom#10931.)

@johnhaley81
Copy link
Collaborator

Yeah, I swore I commented on that exact line saying "These need to be freed" but I can't find it now. /shrug

@implausible
Copy link
Member

Oh wow, good catch. Thanks!

@implausible
Copy link
Member

That should take care of the problem, the intention of this C++ rewrite was to do exactly that, free the patches after we use them. I definitely accidentally a line.

@joshaber
Copy link
Collaborator Author

Verified this fixes the issue we were seeing in Atom 👍

@joshaber joshaber changed the title [WIP] Let my patches go Let my patches go Feb 29, 2016
@johnhaley81
Copy link
Collaborator

👍 Thanks @joshaber

johnhaley81 added a commit that referenced this pull request Feb 29, 2016
@johnhaley81 johnhaley81 merged commit e4bf2f2 into master Feb 29, 2016
@johnhaley81 johnhaley81 deleted the let-my-patches-go branch February 29, 2016 22:17
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.

calculating convenient patches locks files without autocrlf

3 participants

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