Skip to content

Conversation

@ethomson
Copy link
Member

Copy link
Member

Choose a reason for hiding this comment

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

shoudn't this return an int?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Fixed.

@Therzok
Copy link
Member

Therzok commented Dec 10, 2014

I see @ethomson , didn't know! Thanks! I said I needed extra 👀 heh.

Copy link
Member

Choose a reason for hiding this comment

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

Nuke pls.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nuked.

Copy link
Member

Choose a reason for hiding this comment

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

Let's stop this dance. How about adding a dedicated .targets (à la Kudu)?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's clever. I'd like to address this in a separate PR, if you don't mind, but I think it's a wonderful idea.

@ethomson
Copy link
Member Author

@Therzok No problem - I went ahead and opened a new PR up because I'm being impatient. Hope you don't mind! :)

@Therzok
Copy link
Member

Therzok commented Dec 10, 2014

Not at all, I was not here when you opened it. :D As long as it gets merged, I'm one happy person.

@Therzok
Copy link
Member

Therzok commented Dec 10, 2014

Looks great, win64 passed, linux/mac passed, :shipit:

@ethomson
Copy link
Member Author

Rebased on vnext.

@nulltoken nulltoken merged commit 8c48d39 into libgit2:vNext Dec 11, 2014
@nulltoken nulltoken added this to the v0.21 milestone Dec 11, 2014
@nulltoken
Copy link
Member

❤️

@ethomson
Copy link
Member Author

👍 @Therzok - thanks a million.

@Therzok
Copy link
Member

Therzok commented Dec 11, 2014

Now we can get things rolling. I think we still need the TODO from my PR.

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.

3 participants