Keep property names that are sub-clients singular

[M-Zuber]

trafficstars

The convention is that a client that has sub-clients uses a singular form for the property name eg

 IObservableTeamsClient Team { get; }

and not

 IObservableTeamsClient Teams { get; }

Is this a check that could be added into the convention tests? (maybe using something like Humanizer - which might enable us to even just put it in FormatCode and just fix it automagically)

EDIT: we can use this issue to track what places currently need fixing

[shiftkey]

I'm pretty sure this was something I was tracking as part of auditing everything in https://github.com/octokit/octokit.net/pull/1038/ but I'd need to remember just what past me was doing there...

[M-Zuber]

Yeah, I've had the email for that issue pinned since it was opened - because some day I will download the code and do a real review :laughing:

[ryangribble]

I just did a quick search through the code base and found a few properties of clients that are not singular

\Octokit\Clients\IActivitiesClient.cs(14):    IEventsClient Events { get; }
\Octokit\Clients\IActivitiesClient.cs(29):    IFeedsClient Feeds { get; }
\Octokit\Clients\IActivitiesClient.cs(34):    INotificationsClient Notifications { get; }

\Octokit\Clients\IIssuesClient.cs(25):        IIssuesEventsClient Events { get; }
\Octokit\Clients\IIssuesClient.cs(35):        IIssuesLabelsClient Labels { get; }

\Octokit\Clients\IRepositoriesClient.cs(33):  IRepositoryCommentsClient RepositoryComments { get; }
\Octokit\Clients\IRepositoriesClient.cs(49):  IRepositoryDeployKeysClient DeployKeys { get; }
\Octokit\Clients\IRepositoriesClient.cs(215): IRepositoryHooksClient Hooks { get; }
\Octokit\Clients\IRepositoriesClient.cs(221): IRepositoryForksClient Forks { get; }

\Octokit\Clients\IUsersClient.cs(28):         IUserKeysClient Keys { get; }
\Octokit\Clients\IUsersClient.cs(59):         IFollowersClient Followers { get; }

These 2 are also potenitally "awkwardly" named, although the property name does match the API doc, and im not sure if a "better" naming for these "action" based -ing words exists!

\Octokit\Clients\IActivitiesClient.cs(19):    IStarredClient Starring { get; }
\Octokit\Clients\IActivitiesClient.cs(24):    IWatchedClient Watching { get; }

[M-Zuber]

These 2 are also potentially "awkwardly" named

IMO The client name is a bit off. If our gospel is the API docs, they should be

IStarringClient
IWatchingClient

Either way Watch and Star can also be adjective-iy so

\Octokit\Clients\IActivitiesClient.cs(19):    IStarredClient Star { get; }
\Octokit\Clients\IActivitiesClient.cs(24):    IWatchedClient Watch { get; }

would not be so jarring ( :stuck_out_tongue_closed_eyes: ) IMO

In the same vein the StarredRequest class name feels off to me

[devkhan]

The Migration API has an internal Enterprise Migrations client, so that causes troubles if I follow this. As I can't name both of them Migration, so I have named the internal one as IMigrationsClient Migrations and IMigrationClient. The only difference is the plurality. What should be done in such a case?

Here is the explaination by @ryangribble.

Keeping this aside, I feel that since we follow the GitHub API, should we also make clients nested as in the docs and make the sub-clients as internal namespcaces. This will also solve the problem of Migrations API as we can then have something like Octokit.Migration.IMigration Migration, keeping them singular. Is something like this even possible?

[github-actions[bot]]

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!