Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move FindMergeBase() overloads to ObjectDatabase #957

Merged
merged 1 commit into from
Feb 12, 2015
Merged

Move FindMergeBase() overloads to ObjectDatabase #957

merged 1 commit into from
Feb 12, 2015

Conversation

bording
Copy link
Member

@bording bording commented Feb 12, 2015

Per isssues #864 and #951, the overloads of FindMergeBase() have been moved
from CommitLog to ObjectDatabase.

The methods on ICommitLog and CommitLog have been deprecated, and calls to
the CommitLog methods have been changed to call the ObjectDatabase versions
instead.

The relevant tests have been updated as well.

@bording
Copy link
Member Author

bording commented Feb 12, 2015

There were a couple things I wasn't completely sure about:

  • The methods weren't virtual on CommitLog, but all of ObjectDatabase's public methods appeared to be, so I made them virtual too.
  • I went ahead and added Obsolete attributes to the old methods since issue Move FindMergeBase() overloads to ObjectDatabase #951 mentioned deprecating them. I wasn't sure if that was intended or not. I modeled the warning message after the other one I found in the code.

I fixed up all the tests that broke when I made these changes, but I didn't add any new ones because I was assuming getting the existing tests working again would be sufficient.

@nulltoken nulltoken added this to the v0.22 milestone Feb 12, 2015
@@ -14,12 15,14 @@ public interface IQueryableCommitLog : ICommitLog
/// <returns>A list of commits, ready to be enumerated.</returns>
ICommitLog QueryBy(CommitFilter filter);


Copy link
Member

Choose a reason for hiding this comment

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

Ooooh.... A newline sneaked in here 👹 :trollface:

@nulltoken
Copy link
Member

@bording Congrats on your first PR! That looks like a amazing work to me.

I you feel like amending your commit and force pushing to drop the newline, that'd be perfect. Otherwise, I'm happy to merge it as is. We'll clean this later.

@bording
Copy link
Member Author

bording commented Feb 12, 2015

Ack, not sure how I missed that extra newline! :)

I don't mind amending the commit and getting rid of it.

I also noticed that there have been some commits to vNext that are ahead of my branch. Would you prefer I leave my branch where it is, or should I go ahead and rebase to the new vNext?

@nulltoken
Copy link
Member

@bording Wow. You're aklready a git ninja! Awesome.

Rebasing would be better as it tends to make the history easier to understand.

Per isssues #864 and #951, the overloads of FindMergeBase() have been moved
from CommitLog to ObjectDatabase.

The methods on ICommitLog and CommitLog have been deprecated, and calls to
the CommitLog methods have been changed to call the ObjectDatabase versions
instead.

The relevant tests have been updated as well.

Fixes #864 #951
@bording
Copy link
Member Author

bording commented Feb 12, 2015

There you go, newline removed and rebased to latest vNext!

I tend to prefer rebasing when possible, because I hate seeing merge commits in the history. I'm usually using gitk --all and it makes things way more complicated!

@nulltoken nulltoken merged commit 24dfd15 into libgit2:vNext Feb 12, 2015
@nulltoken
Copy link
Member

@bording ✨ ✨ ✨

@nulltoken
Copy link
Member

Published in prerelease NuGet package LibGit2Sharp.0.22.0-pre20150212180712.nupkg

@bording bording deleted the moveFindMergeBase branch February 20, 2015 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants