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.

Sign up for GitHub

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

Changed repos to a specialized collection #842

Merged
merged 3 commits into from
Jul 20, 2015
Merged

Changed repos to a specialized collection #842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bd0447
Changed repos to a specialized collection
khellang Jul 19, 2015
f98ff15
Added overloads with a single string
khellang Jul 19, 2015
43feb74
Changed collection to inherit from Collection<string>
khellang Jul 20, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Octokit.Tests.Integration/Clients/SearchClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ public async Task SearchForFunctionInCode()
public async Task SearchForWordInCode()
{
var request = new SearchIssuesRequest("windows");
request.Repos = new Collection<string> {
"aspnet/dnx",
"aspnet/dnvm"
request.Repos = new RepositoryCollection {
{ "aspnet", "dnx" },
{ "aspnet", "dnvm" }
};

request.SortField = IssueSearchSort.Created;
Expand All @@ -64,7 +64,7 @@ public async Task SearchForWordInCode()
public async Task SearchForOpenIssues()
{
var request = new SearchIssuesRequest("phone");
request.Repos.Add("caliburn-micro/caliburn.micro");
request.Repos.Add("caliburn-micro", "caliburn.micro");
request.State = ItemState.Open;

var issues = await _gitHubClient.Search.SearchIssues(request);
Expand All @@ -91,7 +91,7 @@ public async Task SearchForAllIssuesWithouTaskUsingTerm()
public async Task SearchForAllIssuesUsingTerm()
{
var request = new SearchIssuesRequest("phone");
request.Repos.Add("caliburn-micro/caliburn.micro");
request.Repos.Add("caliburn-micro", "caliburn.micro");

var issues = await _gitHubClient.Search.SearchIssues(request);

Expand Down
8 changes: 4 additions & 4 deletions Octokit.Tests/Clients/SearchClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ public void TestingTheRepoQualifier()
var connection = Substitute.For<IApiConnection>();
var client = new SearchClient(connection);
var request = new SearchIssuesRequest("something");
request.Repos.Add("octokit/octokit.net");
request.Repos.Add("octokit", "octokit.net");

client.SearchIssues(request);

Expand All @@ -1167,8 +1167,8 @@ public async Task ErrorOccursWhenSpecifyingInvalidFormatForRepos()
var client = new SearchClient(connection);

var request = new SearchIssuesRequest("windows");
request.Repos = new Collection<string> {
"haha-business"
request.Repos = new RepositoryCollection {
{ "haha-business", "some&name" }
};

request.SortField = IssueSearchSort.Created;
Expand Down Expand Up @@ -1512,7 +1512,7 @@ public async Task ErrorOccursWhenSpecifyingInvalidFormatForRepos()
var client = new SearchClient(connection);

var request = new SearchCodeRequest("windows");
request.Repos = new Collection<string> {
request.Repos = new RepositoryCollection {
"haha-business"
};

Expand Down
8 changes: 3 additions & 5 deletions Octokit/Models/Request/SearchCodeRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class SearchCodeRequest : BaseSearchRequest
{
public SearchCodeRequest(string term) : base(term)
{
Repos = new Collection<string>();
Repos = new RepositoryCollection();
}

public SearchCodeRequest(string term, string owner, string name)
Expand All @@ -27,9 +27,7 @@ public SearchCodeRequest(string term, string owner, string name)
Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
Ensure.ArgumentNotNullOrEmptyString(name, "name");

var repo = string.Format(CultureInfo.InvariantCulture, "{0}/{1}", owner, name);

Repos.Add(repo);
Repos.Add(owner, name);
}

/// <summary>
Expand Down Expand Up @@ -123,7 +121,7 @@ public IEnumerable<CodeInQualifier> In
/// https://help.github.com/articles/searching-code#users-organizations-and-repositories
/// </remarks>
[SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")]
public Collection<string> Repos { get; set; }
public RepositoryCollection Repos { get; set; }

[SuppressMessage("Microsoft.Globalization", "CA1304:SpecifyCultureInfo", MessageId = "System.String.ToLower")]
public override IReadOnlyList<string> MergedQualifiers()
Expand Down
36 changes: 30 additions & 6 deletions Octokit/Models/Request/SearchIssuesRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class SearchIssuesRequest : BaseSearchRequest
/// </summary>
public SearchIssuesRequest()
{
Repos = new Collection<string>();
Repos = new RepositoryCollection();
}

/// <summary>
Expand All @@ -29,7 +29,7 @@ public SearchIssuesRequest()
/// <param name="term">The term to filter on</param>
public SearchIssuesRequest(string term) : base(term)
{
Repos = new Collection<string>();
Repos = new RepositoryCollection();
}

[Obsolete("this will be deprecated in a future version")]
Expand All @@ -39,9 +39,7 @@ public SearchIssuesRequest(string term, string owner, string name)
Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
Ensure.ArgumentNotNullOrEmptyString(name, "name");

var repo = string.Format(CultureInfo.InvariantCulture, "{0}/{1}", owner, name);

Repos.Add(repo);
Repos.Add(owner, name);
}

/// <summary>
Expand Down Expand Up @@ -196,7 +194,7 @@ public IEnumerable<string> Labels
public string User { get; set; }

[SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")]
public Collection<string> Repos { get; set; }
public RepositoryCollection Repos { get; set; }

public override IReadOnlyList<string> MergedQualifiers()
{
Expand Down Expand Up @@ -337,4 +335,30 @@ public enum IssueTypeQualifier
[Parameter(Value = "issue")]
Issue
}

public class RepositoryCollection : Collection<string>
{
public void Add(string owner, string name)
Copy link
Member

Choose a reason for hiding this comment

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

Fun fact - you can also have Add(string nameWithOwner) here and both sorts will happily work with the collection initializer syntax.

@haacked should we support both? I still like the succinctness of writing them inline rather than splitting them out...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually had both in there, but decided to delete the overload. I guess it doesn't hurt to have it in, since we still do the same, full, validation 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a commit with the single string overloads, you can just strip them out if you decide you don't want both.

{
Add(GetRepositoryName(owner, name));
}

public bool Contains(string owner, string name)
{
return Contains(GetRepositoryName(owner, name));
}

public bool Remove(string owner, string name)
{
return Remove(GetRepositoryName(owner, name));
}

private static string GetRepositoryName(string owner, string name)
{
Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
Ensure.ArgumentNotNullOrEmptyString(name, "name");

return string.Format(CultureInfo.InvariantCulture, "{0}/{1}", owner, name);
}
}
}