Skip to content

Remote deletion#2313

Merged
vmg merged 4 commits intodevelopmentfrom
cmn/remote-delete
May 16, 2014
Merged

Remote deletion#2313
vmg merged 4 commits intodevelopmentfrom
cmn/remote-delete

Conversation

@carlosmn
Copy link
Member

This is based off of #1199

It's been a long time coming, but here's a version of git_remote_delete() that performs all of the steps.

@carlosmn
Copy link
Member Author

I've no idea wtf is up with travis. It looks like the test suite is failing to copy over the test resources.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be more defensive here? strchr may return a null pointer.

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 something like that happens, we're screwed either way, because the config machinery is giving us complete nonsense back.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather return a NULL than segfault. If you're assuming that the config variable will have a ., then assert it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright.

@vmg
Copy link
Member

vmg commented Apr 30, 2014

Travis has been failing a lot but I don't think we've performed any changes in the suite lately. Let me ask the Travis CI guys...

@carlosmn
Copy link
Member Author

It's only the third one down which fails, something about not being able to copy files... really odd.

This should make it more readable and allocate a bunch fewer strings.
When we delete a remote, we also need to go through its fetch refspecs
and remove the references they create locally.
@carlosmn
Copy link
Member Author

Anything else here? The source of travis failures was fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Once

vmg pushed a commit that referenced this pull request May 16, 2014
@vmg vmg merged commit 228272e into development May 16, 2014
@vmg
Copy link
Member

vmg commented May 16, 2014

I like it. :D

@carlosmn carlosmn deleted the cmn/remote-delete branch May 17, 2014 19:57
phatblat pushed a commit to phatblat/libgit2 that referenced this pull request Sep 13, 2014
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.

5 participants