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

chore: update docstrings to follow best practices #284

Merged
merged 6 commits into from
Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
12 changes: 3 additions & 9 deletions src/Momento.Sdk/Auth/EnvMomentoTokenProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,11 @@ namespace Momento.Sdk.Auth;
/// </summary>
public class EnvMomentoTokenProvider : ICredentialProvider
{
/// <summary>
/// JWT provided by user
/// </summary>
/// <inheritdoc cref="Momento.Sdk.Auth.ICredentialProvider.AuthToken" />
public string AuthToken { get; private set; }
/// <summary>
/// Control endpoint extracted from JWT
/// </summary>
/// <inheritdoc cref="Momento.Sdk.Auth.ICredentialProvider.ControlEndpoint" />
public string ControlEndpoint { get; private set; }
/// <summary>
/// Cache endpoint extracted from JWT
/// </summary>
/// <inheritdoc cref="Momento.Sdk.Auth.ICredentialProvider.CacheEndpoint" />
public string CacheEndpoint { get; private set; }

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions src/Momento.Sdk/Auth/ICredentialProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ namespace Momento.Sdk.Auth;
using System.Collections.Generic;

/// <summary>
/// Interface for credentials
/// Specifies the fields that are required for a Momento client to connect to and authenticate with the Momento service.
/// </summary>
public interface ICredentialProvider
{
Expand All @@ -12,11 +12,11 @@ public interface ICredentialProvider
/// </summary>
string AuthToken { get; }
/// <summary>
/// Control endpoint
/// The host which the Momento client will connect to the Momento control plane
/// </summary>
string ControlEndpoint { get; }
/// <summary>
/// Cache endpoint
/// The host which the Momento client will connect to the Momento data plane
/// </summary>
string CacheEndpoint { get; }
}
45 changes: 12 additions & 33 deletions src/Momento.Sdk/Config/Configuration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,24 @@ namespace Momento.Sdk.Config;
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration" />
public class Configuration : IConfiguration
{
/// <inheritdoc cref="Microsoft.Extensions.Logging.ILoggerFactory" />
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.LoggerFactory" />
public ILoggerFactory LoggerFactory { get; }
/// <inheritdoc cref="Momento.Sdk.Config.Retry.IRetryStrategy" />
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.RetryStrategy" />
public IRetryStrategy RetryStrategy { get; }
/// <inheritdoc cref="Momento.Sdk.Config.Middleware.IMiddleware" />
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.Middlewares" />
public IList<IMiddleware> Middlewares { get; }
/// <inheritdoc cref="Momento.Sdk.Config.Transport.ITransportStrategy" />
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.TransportStrategy" />
public ITransportStrategy TransportStrategy { get; }

/// <summary>
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration" />
/// </summary>
/// <param name="retryStrategy">Defines a contract for how and when to retry a request</param>
/// <param name="transportStrategy">This is responsible for configuring network tunables.</param>
/// <param name="loggerFactory">This is responsible for configuraing logging.</param>
public Configuration(IRetryStrategy retryStrategy, ITransportStrategy transportStrategy, ILoggerFactory? loggerFactory = null)
: this(retryStrategy, new List<IMiddleware>(), transportStrategy, loggerFactory)
{

}

/// <summary>
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration" />
/// Create a new instance of Configuration obejct with provided arguments: <see cref="Momento.Sdk.Config.IConfiguration.RetryStrategy">, <see cref="Momento.Sdk.Config.IConfiguration.Middlewares">, <see cref="Momento.Sdk.Config.IConfiguration.TransportStrategy"/>, and <see cref="Momento.Sdk.Config.IConfiguration.LoggerFactory"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

These see tags need to be closed: <see cref="foo" />

/// </summary>
/// <param name="retryStrategy">Defines a contract for how and when to retry a request</param>
/// <param name="middlewares">The Middleware interface allows the Configuration to provide a higher-order function that wraps all requests.</param>
Expand All @@ -52,48 +47,32 @@ public Configuration(IRetryStrategy retryStrategy, IList<IMiddleware> middleware
this.TransportStrategy = transportStrategy;
}

/// <summary>
/// Configures logging
/// </summary>
/// <param name="loggerFactory">This is responsible for configuraing logging.</param>
/// <returns>Configuration object with custom logging provided</returns>
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.WithLoggerFactory(ILoggerFactory)" />
public IConfiguration WithLoggerFactory(ILoggerFactory loggerFactory)
{
return new Configuration(RetryStrategy, Middlewares, TransportStrategy, loggerFactory);
}

/// <summary>
/// Configures retry strategy
/// </summary>
/// <param name="retryStrategy">Defines a contract for how and when to retry a request</param>
/// <returns>Configuration object with custom retry strategy provided</returns>
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.WithRetryStrategy(IRetryStrategy)" />
public IConfiguration WithRetryStrategy(IRetryStrategy retryStrategy)
{
return new Configuration(retryStrategy, Middlewares, TransportStrategy, LoggerFactory);
}

/// <summary>
/// Configures middlewares
/// </summary>
/// <param name="middlewares">The Middleware interface allows the Configuration to provide a higher-order function that wraps all requests.</param>
/// <returns>Configuration object with custom middlewares provided</returns>
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.WithMiddlewares(IList{IMiddleware})" />
public IConfiguration WithMiddlewares(IList<IMiddleware> middlewares)
{
return new Configuration(RetryStrategy, middlewares, TransportStrategy, LoggerFactory);
}

/// <summary>
/// Configures transport trategy
/// </summary>
/// <param name="transportStrategy">This is responsible for configuring network tunables.</param>
/// <returns>Configuration object with custom transport strategy provided</returns>
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.WithTransportStrategy(ITransportStrategy)" />
public IConfiguration WithTransportStrategy(ITransportStrategy transportStrategy)
{
return new Configuration(RetryStrategy, Middlewares, transportStrategy, LoggerFactory);
}

/// <summary>
/// Configures middlewares
/// Add the specified middlewares to an existing instance of Configuration object in addition to already specified middlewares.
/// </summary>
/// <param name="additionalMiddlewares">The Middleware interface allows the Configuration to provide a higher-order function that wraps all requests.</param>
/// <returns>Configuration object with custom middlewares provided</returns>
Expand All @@ -108,9 +87,9 @@ public Configuration WithAdditionalMiddlewares(IList<IMiddleware> additionalMidd
}

/// <summary>
/// Configures client timeout for transport strategy
/// Add the specified client timeout to an existing instance of Configuration object as an addiion to the existing transport strategy.
/// </summary>
/// <param name="clientTimeoutMillis">Client timeout in milliseconds.</param>
/// <param name="clientTimeoutMillis">The amount of time to wait before cancelling the request.</param>
/// <returns>Configuration object with client timeout provided</returns>
public Configuration WithClientTimeoutMillis(uint clientTimeoutMillis)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Momento.Sdk/Config/Configurations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
namespace Momento.Sdk.Config;

/// <summary>
/// Provide pre-build configurations.
/// Provide pre-built configurations.
/// </summary>
public class Configurations
{
Expand Down
33 changes: 32 additions & 1 deletion src/Momento.Sdk/Config/IConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,47 @@ namespace Momento.Sdk.Config;
/// </summary>
public interface IConfiguration
{
/// <inheritdoc cref="Momento.Sdk.Config.IConfiguration.LoggerFactory" />
public ILoggerFactory LoggerFactory { get; }
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it's trying to inherit from itself. This should (I think) reference Microsoft.Extensions.Logging.ILoggerFactory instead, but I'm wondering if these crefs are necessary at all? It looked like <inheritdoc /> was working fine without that attribute, and I think @malandis was suggesting that usage in his comment (please correct me if I'm wrong about that).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing it out!
I moved the summaries to the interfaces and forgot to remove cref. I will remove them.


/// <inheritdoc cref="Momento.Sdk.Config.Retry.IRetryStrategy" />
public IRetryStrategy RetryStrategy { get; }
/// <inheritdoc cref="Momento.Sdk.Config.Middleware.IMiddleware" />
public IList<IMiddleware> Middlewares { get; }
/// <inheritdoc cref="Momento.Sdk.Config.Transport.ITransportStrategy" />
public ITransportStrategy TransportStrategy { get; }

/// <summary>
/// Creates a new instance of the Configuration object, updated to use the specified logger factory.
/// </summary>
/// <param name="loggerFactory">This is responsible for configuraing logging.</param>
/// <returns>Configuration object with custom logging provided</returns>
public IConfiguration WithLoggerFactory(ILoggerFactory loggerFactory);

/// <summary>
/// Creates a new instance of the Configuration object, updated to use the specified retry strategy.
/// </summary>
/// <param name="retryStrategy">Defines a contract for how and when to retry a request</param>
/// <returns>Configuration object with custom retry strategy provided</returns>
public IConfiguration WithRetryStrategy(IRetryStrategy retryStrategy);

/// <summary>
/// Creates a new instance of the Configuration object, updated to use the specified middlewares.
/// </summary>
/// <param name="middlewares">The Middleware interface allows the Configuration to provide a higher-order function that wraps all requests.</param>
/// <returns>Configuration object with custom middlewares provided</returns>
public IConfiguration WithMiddlewares(IList<IMiddleware> middlewares);

/// <summary>
/// Creates a new instance of the Configuration object, updated to use the specified transport strategy.
/// </summary>
/// <param name="transportStrategy">This is responsible for configuring network tunables.</param>
/// <returns>Configuration object with custom transport strategy provided</returns>
public IConfiguration WithTransportStrategy(ITransportStrategy transportStrategy);

/// <summary>
/// Creates a new instance of the Configuration object, updated to use the specified client timeout.
/// </summary>
/// <param name="clientTimeoutMillis">The amount of time to wait before cancelling the request.</param>
/// <returns>Configuration object with custom client timeout provided</returns>
public IConfiguration WithClientTimeoutMillis(uint clientTimeoutMillis);
}