Skip to content

Add IBelongToARepository #806

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

Merged
merged 2 commits into from
Sep 6, 2014
Merged

Add IBelongToARepository #806

merged 2 commits into from
Sep 6, 2014

Conversation

dahlbyk
Copy link
Member

@dahlbyk dahlbyk commented Aug 31, 2014

As discussed in #574 and #665, here's an interface that allows access to an instance's corresponding IRepository for certain (non-standard) usage patterns.

TODO:

  • Add a meta test that the interface is only ever explicitly implemented

@nulltoken
Copy link
Member

Do we need to implement the interface for every derived types once the base ones expose it?

Note: LibGit2SharpPublicInterfacesCoverAllPublicMembers seems to be failing.

@nulltoken nulltoken added this to the v0.20 milestone Aug 31, 2014
/// <param name="canonicalName">The canonical name.</param>
/// <param name="targetIdentifier">The target identifier.</param>
protected Reference(string canonicalName, string targetIdentifier)
protected Reference(IRepository repo, string canonicalName, string targetIdentifier)
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this break the API?

Copy link
Member Author

Choose a reason for hiding this comment

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

So it does, though this ctor was never meant for public consumption. I've marked this one as [Obsolete] in favor of an internal one with the new signature.

@dahlbyk
Copy link
Member Author

dahlbyk commented Sep 1, 2014

Remove unused Handlers namespace

On closer inspection, the namespace is not actually unused...R# got confused.

@dahlbyk
Copy link
Member Author

dahlbyk commented Sep 1, 2014

Fixed and added new meta test. Failure looks like this:

GitObject has public method get_Repository which should be explicitly implemented.

@dahlbyk dahlbyk force-pushed the IBelong branch 2 times, most recently from 0549b3a to ee0d0b7 Compare September 1, 2014 03:39
@dahlbyk
Copy link
Member Author

dahlbyk commented Sep 1, 2014

So my first pass at this was basically terrible. Trying again, with some sanity check tests for good measure.

/// </summary>
IRepository Repository { get; }
}
}
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 take a moment of silence to reflect on all the good things POSIX has brought us :trollface:

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@nulltoken
Copy link
Member

I don't hear anyone strongly opposing to this. Let's move it in. Could you please rebase this PR?

@dahlbyk
Copy link
Member Author

dahlbyk commented Sep 5, 2014

Could you please rebase this PR?

Done

@nulltoken
Copy link
Member

It's a go! 👍

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