Remove usage of optional arguments to public methods #779

[JPaulDuncan]

No description provided.

[nulltoken]

Thanks!

Looks very clean to me. I'll take a deeper look tomorrow though and will post my comments.

[nulltoken]

@haacked This PR fixes issue #779 that you raised. Maybe would you be interested in reviewing it as well.

[nulltoken]

@JPaulDuncan Below some change proposals. Mainly making sure the extensions methods expect an interface rather than a concrete type. Some other changes resolve ambiguous types in xml doc.

Two additional recommendations:

• It's usually better to create a new branch for a PR rather than modifying your own vNext.
• Could you please reword your commit message so that the first line of it doesn't exceed 50 chars? See this Makes RemoteUpdater disposable so remotehandle can be used throughout an... #807 (comment) for more details.

Thanks!

diff --git a/LibGit2Sharp/RepositoryExtensions.cs b/LibGit2Sharp/RepositoryExtensions.cs
index b401185..90b3c68 100644
--- a/LibGit2Sharp/RepositoryExtensions.cs
+++ b/LibGit2Sharp/RepositoryExtensions.cs
@@ -447,7 +447,7 @@ internal static ObjectId Committish(this Repository repo, object identifier)
         /// <param name="branch">The branch to merge into the branch pointed at by HEAD.</param>
         /// <param name="merger">The <see cref="Signature"/> of who is performing the merge.</param>
         /// <returns>The <see cref="MergeResult"/> of the merge.</returns>
-        public static MergeResult Merge(this Repository repository, Branch branch, Signature merger)
+        public static MergeResult Merge(this IRepository repository, Branch branch, Signature merger)
         {
             return repository.Merge(branch, merger, null);
         }
@@ -455,25 +455,27 @@ public static MergeResult Merge(this Repository repository, Branch branch, Signa
         /// <summary>
         /// Merges changes from the commit into the branch pointed at by HEAD.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="committish">The commit to merge into the branch pointed at by HEAD.</param>
         /// <param name="merger">The <see cref="Signature"/> of who is performing the merge.</param>
         /// <returns>The <see cref="MergeResult"/> of the merge.</returns>
-        public static MergeResult Merge(this Repository repository, string committish, Signature merger)
+        public static MergeResult Merge(this IRepository repository, string committish, Signature merger)
         {
             return repository.Merge(committish, merger, null);
         }

         /// <summary>
-        /// Checkout the tip commit of the specified <see cref="Branch"/> object. If this commit is the
-        /// current tip of the branch, will checkout the named branch. Otherwise, will checkout the tip commit
-        /// as a detached HEAD.
+        /// Checkout the commit pointed at by the tip of the specified <see cref="Branch"/>.
+        /// <para>
+        ///   If this commit is the current tip of the branch as it exists in the repository, the HEAD
+        ///   will point to this branch. Otherwise, the HEAD will be detached, pointing at the commit sha.
+        /// </para>
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="branch">The <see cref="Branch"/> to check out.</param>
         /// <param name="options"><see cref="CheckoutOptions"/> controlling checkout behavior.</param>
         /// <returns>The <see cref="Branch"/> that was checked out.</returns>
-        public static Branch Checkout(this Repository repository, Branch branch, CheckoutOptions options)
+        public static Branch Checkout(this IRepository repository, Branch branch, CheckoutOptions options)
         {
             return repository.Checkout(branch, options, null);
         }
@@ -484,11 +486,11 @@ public static Branch Checkout(this Repository repository, Branch branch, Checkou
         ///   Will detach the HEAD and make it point to this commit sha.
         /// </para>
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="commit">The <see cref="LibGit2Sharp.Commit"/> to check out.</param>
         /// <param name="options"><see cref="CheckoutOptions"/> controlling checkout behavior.</param>
         /// <returns>The <see cref="Branch"/> that was checked out.</returns>
-        public static Branch Checkout(this Repository repository, Commit commit, CheckoutOptions options)
+        public static Branch Checkout(this IRepository repository, Commit commit, CheckoutOptions options)
         {
             return repository.Checkout(commit, options, null);
         }
@@ -500,11 +502,11 @@ public static Branch Checkout(this Repository repository, Commit commit, Checkou
         ///   will point to the branch. Otherwise, the HEAD will be detached, pointing at the commit sha.
         /// </para>
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="committishOrBranchSpec">A revparse spec for the commit or branch to checkout.</param>
         /// <param name="options"><see cref="CheckoutOptions"/> controlling checkout behavior.</param>
         /// <returns>The <see cref="Branch"/> that was checked out.</returns>
-        public static Branch Checkout(this Repository repository, string committishOrBranchSpec, CheckoutOptions options)
+        public static Branch Checkout(this IRepository repository, string committishOrBranchSpec, CheckoutOptions options)
         {
             return repository.Checkout(committishOrBranchSpec, options, null);
         }
@@ -515,22 +517,22 @@ public static Branch Checkout(this Repository repository, string committishOrBra
         /// This method does not switch branches or update the current repository HEAD.
         /// </para>
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name = "committishOrBranchSpec">A revparse spec for the commit or branch to checkout paths from.</param>
         /// <param name="paths">The paths to checkout. Will throw if null is passed in. Passing an empty enumeration results in nothing being checked out.</param>
-        public static void CheckoutPaths(this Repository repository, string committishOrBranchSpec, IEnumerable<string> paths)
+        public static void CheckoutPaths(this IRepository repository, string committishOrBranchSpec, IEnumerable<string> paths)
         {
             repository.CheckoutPaths(committishOrBranchSpec, paths, null);
         }

         /// <summary>
-        /// Sets the current <see cref="Head"/> to the specified commit and optionally resets the <see cref="Index"/> and
+        /// Sets the current <see cref="IRepository.Head"/> to the specified commit and optionally resets the <see cref="Index"/> and
         /// the content of the working tree to match.
         /// </summary>
         /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="resetMode">Flavor of reset operation to perform.</param>
         /// <param name="commit">The target commit object.</param>
-        public static void Reset(this Repository repository, ResetMode resetMode, Commit commit)
+        public static void Reset(this IRepository repository, ResetMode resetMode, Commit commit)
         {
             repository.Reset(resetMode, commit, null, null);
         }
@@ -541,7 +543,7 @@ public static void Reset(this Repository repository, ResetMode resetMode, Commit
         /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="commit">The target commit object.</param>
         /// <param name="paths">The list of paths (either files or directories) that should be considered.</param>
-        public static void Reset(this Repository repository, Commit commit, IEnumerable<string> paths)
+        public static void Reset(this IRepository repository, Commit commit, IEnumerable<string> paths)
         {
             repository.Reset(commit, paths, null);
         }
@@ -549,9 +551,9 @@ public static void Reset(this Repository repository, Commit commit, IEnumerable<
         /// <summary>
         /// Replaces entries in the <see cref="Repository.Index"/> with entries from the specified commit.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="commit">The target commit object.</param>
-        public static void Reset(this Repository repository, Commit commit)
+        public static void Reset(this IRepository repository, Commit commit)
         {
             repository.Reset(commit, null, null);
         }
@@ -574,10 +576,10 @@ public static Commit Commit(this IRepository repository, string message, Signatu
         /// <summary>
         /// Find where each line of a file originated.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="path">Path of the file to blame.</param>
         /// <returns>The blame for the file.</returns>
-        public static BlameHunkCollection Blame(this Repository repository, string path)
+        public static BlameHunkCollection Blame(this IRepository repository, string path)
         {
             return repository.Blame(path, null);
         }
@@ -585,11 +587,11 @@ public static BlameHunkCollection Blame(this Repository repository, string path)
         /// <summary>
         /// Cherry-picks the specified commit.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
-        /// <param name="commit">The <see cref="Commit"/> to cherry-pick.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
+        /// <param name="commit">The <see cref="LibGit2Sharp.Commit"/> to cherry-pick.</param>
         /// <param name="committer">The <see cref="Signature"/> of who is performing the cherry pick.</param>
         /// <returns>The result of the cherry pick.</returns>
-        public static CherryPickResult CherryPick(this Repository repository, Commit commit, Signature committer)
+        public static CherryPickResult CherryPick(this IRepository repository, Commit commit, Signature committer)
         {
             return repository.CherryPick(commit, committer, null);
         }
@@ -597,11 +599,11 @@ public static CherryPickResult CherryPick(this Repository repository, Commit com
         /// <summary>
         /// Merges changes from commit into the branch pointed at by HEAD.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
         /// <param name="commit">The commit to merge into the branch pointed at by HEAD.</param>
         /// <param name="merger">The <see cref="Signature"/> of who is performing the merge.</param>
         /// <returns>The <see cref="MergeResult"/> of the merge.</returns>
-        public static MergeResult Merge(this Repository repository, Commit commit, Signature merger)
+        public static MergeResult Merge(this IRepository repository, Commit commit, Signature merger)
         {
             return repository.Merge(commit, merger, null);
         }
@@ -609,11 +611,11 @@ public static MergeResult Merge(this Repository repository, Commit commit, Signa
         /// <summary>
         /// Revert the specified commit.
         /// </summary>
-        /// <param name="repository">The <see cref="Repository"/> being worked with.</param>
-        /// <param name="commit">The <see cref="Commit"/> to revert.</param>
+        /// <param name="repository">The <see cref="IRepository"/> being worked with.</param>
+        /// <param name="commit">The <see cref="LibGit2Sharp.Commit"/> to revert.</param>
         /// <param name="reverter">The <see cref="Signature"/> of who is performing the revert.</param>
         /// <returns>The result of the revert.</returns>
-        public static RevertResult Revert(this Repository repository, Commit commit, Signature reverter)
+        public static RevertResult Revert(this IRepository repository, Commit commit, Signature reverter)
         {
             return repository.Revert(commit, reverter, null);
         }

[JPaulDuncan]

@nulltoken I've made the recommended changes. As far as the branching strategy, I'll do that on the next one. It's a good idea.

[nulltoken]

Neat! Could you please squash the two commits together and rebase your PR on top of the latest vNext so we can get this one merged?

[nulltoken]

Manually merged in vNext with b8225a7

Thanks!