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

[Approved] Add repository topics #2246

Merged
merged 56 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
220e9a3
ReSharper auto migration
SeanKilleen Sep 8, 2020
1d0cb56
Add read-ony list of topics to repository class
SeanKilleen Sep 8, 2020
3fa7807
Switch to IReadOnlyList<> for immutability
SeanKilleen Sep 8, 2020
56295b3
Add mercy preview to clients returning Task<Repository>
SeanKilleen Sep 8, 2020
21d3bbd
get all topics method
SeanKilleen Sep 8, 2020
4b3e57f
ReplaceAllTopics method
SeanKilleen Sep 8, 2020
697d3d6
Add fields to search request
SeanKilleen Sep 8, 2020
efd373e
Add parameter qualifiers
SeanKilleen Sep 8, 2020
620cb26
Correct my misunderstanding of what Topics represents
SeanKilleen Sep 8, 2020
dd518d3
Add preview header to AcceptHeaders
SeanKilleen Sep 8, 2020
091bff0
Add headers
SeanKilleen Sep 8, 2020
cb5a06c
overloads for repository ID
SeanKilleen Sep 8, 2020
4a178ea
Update the interface
SeanKilleen Sep 8, 2020
af48670
Convert to use ApiOptions
SeanKilleen Sep 8, 2020
e68a5b8
add overloads
SeanKilleen Sep 8, 2020
ad9b819
Observable for GetAllTopics
SeanKilleen Sep 8, 2020
52db2ab
observable implementations for ReplaceAllTopics
SeanKilleen Sep 8, 2020
fca13f4
Use IObservable<string>
SeanKilleen Sep 8, 2020
0b88b7a
Ah, I see -- need to sync up the signatures.
SeanKilleen Sep 8, 2020
1cd3116
Is this the only way to make the signatures match up?
SeanKilleen Sep 8, 2020
456f431
Some integration tests
SeanKilleen Sep 9, 2020
ebbe318
Add RepositoryTopics from #1721
SeanKilleen Sep 9, 2020
7d8d0d7
Use RepositoryTopics class
SeanKilleen Sep 9, 2020
95f102a
More signature updates
SeanKilleen Sep 9, 2020
67998dc
Add ReplaceAllTopics test real quick
SeanKilleen Sep 9, 2020
ca9e8e9
Empty list by default
SeanKilleen Sep 9, 2020
7761dbd
integration test updates
SeanKilleen Sep 14, 2020
9b126e5
Additional tests for using repo ID
SeanKilleen Sep 14, 2020
1e7ac1d
Some unit tests & corresponding changes
SeanKilleen Sep 14, 2020
1792546
unit tests for the GetAllTopics method
SeanKilleen Sep 14, 2020
fd7a2be
Unit tests, including a failing one I don't understand
SeanKilleen Sep 14, 2020
0d6da2a
extract client & connection to fields
SeanKilleen Sep 14, 2020
327ee27
oops, bad argumnt
SeanKilleen Sep 14, 2020
c9ed7a7
RepositoryTopics unit tests
SeanKilleen Sep 14, 2020
19ea9fd
Mark integration tests as such
SeanKilleen Sep 14, 2020
27d4d68
Integreation tests for search
SeanKilleen Sep 14, 2020
6068b1b
Some failing tests to fix soon
SeanKilleen Sep 14, 2020
fe182e1
more unit tests
SeanKilleen Sep 15, 2020
b7c6901
Update Octokit.Reactive/Clients/IObservableRepositoriesClient.cs
SeanKilleen Sep 15, 2020
5dba2fd
Update Octokit.Tests.Integration/Clients/SearchClientTests.cs
SeanKilleen Sep 15, 2020
e55273b
Update Octokit.Tests.Integration/Clients/SearchClientTests.cs
SeanKilleen Sep 15, 2020
30b2215
Update Octokit/Clients/IRepositoriesClient.cs
SeanKilleen Sep 15, 2020
cd0f082
Update Octokit/Clients/IRepositoriesClient.cs
SeanKilleen Sep 15, 2020
cdbb96b
Update Octokit.Tests/Clients/SearchClientTests.cs
SeanKilleen Sep 15, 2020
550f4e3
Update Octokit/Clients/RepositoriesClient.cs
SeanKilleen Sep 15, 2020
3e10bc2
Update Octokit/Helpers/ApiUrls.cs
SeanKilleen Sep 15, 2020
a6c49ed
Update Octokit/Clients/RepositoriesClient.cs
SeanKilleen Sep 15, 2020
04161cf
Merge branch 'main' into 1707_add-repository-topics
SeanKilleen Oct 6, 2020
4b62ec0
Merge branch 'main' into 1707_add-repository-topics
shiftkey Feb 21, 2021
257be14
Use helpers and CreateRepositoryContext
SeanKilleen Feb 22, 2021
5fb88ad
Merge branch 'main' into 1707_add-repository-topics
SeanKilleen Feb 22, 2021
8730206
cleanup: explicit private modifier
SeanKilleen Feb 22, 2021
84d6144
cleanup: field names
SeanKilleen Feb 22, 2021
fa94465
cleanup: extract variable
SeanKilleen Feb 22, 2021
c5574ed
cleanup: spacing
SeanKilleen Feb 22, 2021
2bc3df5
Cleanup: usings
SeanKilleen Feb 22, 2021
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
76 changes: 76 additions & 0 deletions Octokit.Reactive/Clients/IObservableRepositoriesClient.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Reactive;
using System.Threading.Tasks;

namespace Octokit.Reactive
{
Expand Down Expand Up @@ -556,5 +558,79 @@ public interface IObservableRepositoriesClient
/// Refer to the API documentation for more information: https://developer.github.com/v3/repos/projects/
/// </remarks>
IObservableProjectsClient Project { get; }

/// <summary>
/// Gets all topics for the specified owner and repository name.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="options">Options for changing the API response</param>
/// <returns>All topics associated with the repository.</returns>
IObservable<string> GetAllTopics(string owner, string name, ApiOptions options);

/// <summary>
/// Gets all topics for the specified owner and repository name.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <returns>All topics associated with the repository.</returns>
IObservable<string> GetAllTopics(string owner, string name);


SeanKilleen marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets all topics for the specified repository ID.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <param name="options">Options for changing the API response</param>
/// <returns>All topics associated with the repository.</returns>
IObservable<string> GetAllTopics(long repositoryId, ApiOptions options);

/// <summary>
/// Gets all topics for the specified repository ID.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <returns>All topics associated with the repository.</returns>
IObservable<string> GetAllTopics(long repositoryId);

/// <summary>
/// Replaces all topics for the specified repository.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#replace-all-repository-topics">API documentation</a> for more details
///
/// This is a replacement operation; it is not additive. To clear repository topics, for example, you could specify an empty list of topics here.
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <param name="topics">The list of topics to associate with the repository</param>
/// <returns>All topics now associated with the repository.</returns>
IObservable<string> ReplaceAllTopics(long repositoryId, IEnumerable<string> topics);

/// <summary>
/// Replaces all topics for the specified repository.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#replace-all-repository-topics">API documentation</a> for more details
///
/// This is a replacement operation; it is not additive. To clear repository topics, for example, you could specify an empty list of topics here.
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="topics">The list of topics to associate with the repository</param>
/// <returns>All topics now associated with the repository.</returns>
IObservable<string> ReplaceAllTopics(string owner, string name, IEnumerable<string> topics);

}
}
93 changes: 93 additions & 0 deletions Octokit.Reactive/Clients/ObservableRepositoriesClient.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reactive;
using System.Reactive.Linq;
using System.Reactive.Threading.Tasks;
using System.Threading.Tasks;
using Octokit.Reactive.Clients;
using Octokit.Reactive.Internal;

Expand Down Expand Up @@ -821,5 +823,96 @@ public IObservable<CompareResult> Compare(string owner, string name, string @bas
/// Refer to the API documentation for more information: https://developer.github.com/v3/repos/projects/
/// </remarks>
public IObservableProjectsClient Project { get; private set; }

/// <summary>
/// Gets all topics for the specified owner and repository name.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="options">Options for changing the API response</param>
/// <returns>All topics associated with the repository.</returns>
public IObservable<string> GetAllTopics(string owner, string name, ApiOptions options)
{
var url = ApiUrls.RepositoryTopics(owner, name);
return _connection.GetAndFlattenAllPages<string>(url, null, AcceptHeaders.RepositoryTopicsPreview, options);
}

/// <summary>
/// Gets all topics for the specified owner and repository name.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <returns>All topics associated with the repository.</returns>
public IObservable<string> GetAllTopics(string owner, string name)
{
return GetAllTopics(owner, name, ApiOptions.None);
}

/// <summary>
/// Gets all topics for the specified repository ID.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <param name="options">Options for changing the API response</param>
/// <returns>All topics associated with the repository.</returns>
public IObservable<string> GetAllTopics(long repositoryId, ApiOptions options)
{
var url = ApiUrls.RepositoryTopics(repositoryId);
return _connection.GetAndFlattenAllPages<string>(url, null, AcceptHeaders.RepositoryTopicsPreview, options);
}

/// <summary>
/// Gets all topics for the specified repository ID.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <returns>All topics associated with the repository.</returns>
public IObservable<string> GetAllTopics(long repositoryId)
{
return GetAllTopics(repositoryId, ApiOptions.None);
}

/// <summary>
/// Replaces all topics for the specified repository.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#replace-all-repository-topics">API documentation</a> for more details
///
/// This is a replacement operation; it is not additive. To clear repository topics, for example, you could specify an empty list of topics here.
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <param name="topics">The list of topics to associate with the repository</param>
/// <returns>All topics now associated with the repository.</returns>
public IObservable<string> ReplaceAllTopics(long repositoryId, IEnumerable<string> topics)
{
return _client.ReplaceAllTopics(repositoryId, topics).Result.ToObservable();
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 do not like this use of .Result. I may be misunderstanding some of the conventions that caused me to wind up in this place.

  • If I tried to make the signature into IObservable<List<string>>, I saw that the signature doesn't match.
  • If I attempt to change the signature to async Task<IObservable<string>>, I see a similar issue.

This was the only way that seemed to satisfy the conventions' requirements, but I'm almost certainly missing something.

Choose a reason for hiding this comment

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

لاداعي✌️

}

/// <summary>
/// Replaces all topics for the specified repository.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#replace-all-repository-topics">API documentation</a> for more details
///
/// This is a replacement operation; it is not additive. To clear repository topics, for example, you could specify an empty list of topics here.
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="topics">The list of topics to associate with the repository</param>
/// <returns>All topics now associated with the repository.</returns>
public IObservable<string> ReplaceAllTopics(string owner, string name, IEnumerable<string> topics)
{
return _client.ReplaceAllTopics(owner, name, topics).Result.ToObservable();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

}
}
}
1 change: 1 addition & 0 deletions Octokit.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ II.2.12 &lt;HandlesEvent /&gt;&#xD;
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpPlaceEmbeddedOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpRenamePlacementToArrangementMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpUseContinuousIndentInsideBracesMigration/@EntryIndexedValue">True</s:Boolean>
shiftkey marked this conversation as resolved.
Show resolved Hide resolved
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAddAccessorOwnerDeclarationBracesMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002ECSharpPlaceAttributeOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
Expand Down
75 changes: 75 additions & 0 deletions Octokit/Clients/IRepositoriesClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -621,5 +621,80 @@ public interface IRepositoriesClient
/// Refer to the API documentation for more information: https://developer.github.com/v3/repos/projects/
/// </remarks>
IProjectsClient Project { get; }

/// <summary>
/// Gets all topics for the specified owner and repository name.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="options">Options for changing the API response</param>
/// <returns>All topics associated with the repository.</returns>
Task<IReadOnlyList<string>> GetAllTopics(string owner, string name, ApiOptions options);

/// <summary>
/// Gets all topics for the specified owner and repository name.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <returns>All topics associated with the repository.</returns>
Task<IReadOnlyList<string>> GetAllTopics(string owner, string name);


Choose a reason for hiding this comment

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

ااقفز

SeanKilleen marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets all topics for the specified repository ID.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <param name="options">Options for changing the API response</param>
/// <returns>All topics associated with the repository.</returns>
Task<IReadOnlyList<string>> GetAllTopics(long repositoryId, ApiOptions options);

/// <summary>
/// Gets all topics for the specified repository ID.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#get-all-repository-topics">API documentation</a> for more details
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <returns>All topics associated with the repository.</returns>
Task<IReadOnlyList<string>> GetAllTopics(long repositoryId);

SeanKilleen marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Replaces all topics for the specified repository.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#replace-all-repository-topics">API documentation</a> for more details
///
/// This is a replacement operation; it is not additive. To clear repository topics, for example, you could specify an empty list of topics here.
/// </remarks>
/// <param name="repositoryId">The ID of the repository</param>
/// <param name="topics">The list of topics to associate with the repository</param>
/// <returns>All topics now associated with the repository.</returns>
Task<IReadOnlyList<string>> ReplaceAllTopics(long repositoryId, IEnumerable<string> topics);

/// <summary>
/// Replaces all topics for the specified repository.
/// </summary>
/// <remarks>
/// See the <a href="https://docs.github.com/en/rest/reference/repos#replace-all-repository-topics">API documentation</a> for more details
///
/// This is a replacement operation; it is not additive. To clear repository topics, for example, you could specify an empty list of topics here.
/// </remarks>
/// <param name="owner">The owner of the repository</param>
/// <param name="name">The name of the repository</param>
/// <param name="topics">The list of topics to associate with the repository</param>
/// <returns>All topics now associated with the repository.</returns>
Task<IReadOnlyList<string>> ReplaceAllTopics(string owner, string name, IEnumerable<string> topics);

}
}
Loading