Skip to content

Remove usage of optional arguments to public methods #779 - #810

Closed
JPaulDuncan wants to merge 4 commits into
libgit2:vNextfrom
JPaulDuncan:vNext
Closed

Remove usage of optional arguments to public methods #779#810
JPaulDuncan wants to merge 4 commits into
libgit2:vNextfrom
JPaulDuncan:vNext

Conversation

@JPaulDuncan

Copy link
Copy Markdown
Contributor

Modifies the IRepository.cs and Repository.cs files to remove the optional parameters from the various method signatures. This will help address potential versioning issues.

Comment thread LibGit2Sharp/IRepository.cs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Original comment is still valid - options is not required.

@dahlbyk

dahlbyk commented Sep 2, 2014

Copy link
Copy Markdown
Member

I agree with @nulltoken (from #779 (comment)):

I'd go with making IRepository expose the methods with the full set of arguments and creating overloads in RepositoryExtensions.cs.

The PR also includes a bunch of reformatting that distracts from the core change.
If you could revert those that would be great.

@JPaulDuncan

Copy link
Copy Markdown
Contributor Author

Fracking Resharper!

Comment thread LibGit2Sharp/Repository.cs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Formatting here.

@JPaulDuncan

Copy link
Copy Markdown
Contributor Author

Christ.

@JPaulDuncan JPaulDuncan closed this Sep 2, 2014
@nulltoken

Copy link
Copy Markdown
Member

@JPaulDuncan Hey! You don't need to close the PR to get it fixed. Just update locally your branch, (force) push and tada.... the PR gets automatically updated!

However, for this to work, you'll have it to reopen it, though 😉

@JPaulDuncan JPaulDuncan reopened this Sep 3, 2014
@nulltoken

Copy link
Copy Markdown
Member

Fix #779

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please remove this declaration from IRepository?

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.

4 participants