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

Documentation Tweak #57

Closed
granadacoder opened this issue Oct 9, 2018 · 3 comments
Closed

Documentation Tweak #57

granadacoder opened this issue Oct 9, 2018 · 3 comments

Comments

@granadacoder
Copy link

granadacoder commented Oct 9, 2018

Hi.

This is great library, and is really helping my team implement Hashicorp Vault

My one comment is that using the "var" shortcut word for variables in the documentation makes reading the documentation harder to pickup easily.

https://github.com/rajanadar/VaultSharp

Example:

var azureCredentials = await vaultClient.V1.Secrets.Azure.GetCredentialsAsync(roleName);
var clientId = azureCredentials.Data.ClientId;
var clientSecret = azureCredentials.Data.ClientSecret;

MUCH MORE PREFERRED FOR DOCUMENTATION

Secret<AzureCredentials> azureCredentials = await ivc.V1.Secrets.Azure.GetCredentialsAsync("myRoleName");
string currentPassword = adCreds.Data.CurrentPassword;
string clientSecret = azureCredentials.Data.ClientSecret;

This really helps a new user (like me) "see" the api documentation much more quickly, then having to actually write example code to see what the real return is.

Like here in the below, I clearly see that TokenAuthMethodInfo is an implementation of IAuthMethodInfo.
And the var word that points to "new VaultClientSettings" isn't hard to discern.

// Initialize one of the several auth methods.
IAuthMethodInfo authMethod = new TokenAuthMethodInfo("MY_VAULT_TOKEN");

// Initialize settings. You can also set proxies, custom delegates etc. here.
var vaultClientSettings = new VaultClientSettings("https://MY_VAULT_SERVER:8200", authMethod);

IVaultClient vaultClient = new VaultClient(vaultClientSettings);

My team's "rule" for the use of var is
"When its clearly a 'new Something', then ok". But when its a result of a method call that isn't easily seen, use the real object type." We do this for code reviews. I'm just looking at code, not necessarily running it, so var in code reviews make it harder to see what is happening, IMHO.

I'm not trying to change your personal preference. However, with documentation and examples, the use of "var" makes learning the API slower, IMHO.

It's a small thing in regards to the overall great library. But would be helpful, IMHO.

Thank you for your consideration.

@rajanadar
Copy link
Owner

Valid point. Thanks for using the library. I'll update the docs in the coming days.

@granadacoder
Copy link
Author

Thanks

@rajanadar
Copy link
Owner

@granadacoder i have updated the docs. next publish will reflect everywhere.

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

No branches or pull requests

2 participants