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

only init when needed #3

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@Modarel
Copy link
Contributor

Modarel commented Mar 4, 2019

No description provided.

@Modarel Modarel requested review from wulfland and BenjaminAbt Mar 4, 2019

@@ -8,23 +8,42 @@ namespace AzDoBridge.Clients
{
public class AzureDevOpsClient
{
protected readonly string Username;

This comment has been minimized.

@BenjaminAbt

BenjaminAbt Mar 4, 2019

Contributor

Use a property for protected, not a field.

@@ -39,18 +39,15 @@ public static class AlexaController
string azureDevopsAccessToken = Environment.GetEnvironmentVariable("AzureDevOps:PAT", EnvironmentVariableTarget.Process);

This comment has been minimized.

@BenjaminAbt

BenjaminAbt Mar 4, 2019

Contributor

Use Option pattern like in other classes.

@@ -8,23 +8,42 @@ namespace AzDoBridge.Clients
{
public class AzureDevOpsClient
{
protected readonly string Username;
protected readonly string PersonalAccessToken;
Lazy<TfsTeamProjectCollection> _tfsTeamProjectCollection;
public AzureDevOpsClient(Uri hostname, string username, string personalAccessToken)

This comment has been minimized.

@BenjaminAbt

BenjaminAbt Mar 4, 2019

Contributor

You should also add an interface and inject this to other classes/methods - not the implementation.
Otherwise it will be not possible to mock that client for tests

@@ -48,6 +49,7 @@ public LaunchRequestHandle(ILogger log) : base(log)
}
}
else { skillResponse = ResponseBuilder.Tell($"Unautherized"); }

This comment has been minimized.

@BenjaminAbt

BenjaminAbt Mar 4, 2019

Contributor

Typo: Unauthorized - but maybe a full sentence as return?

@@ -20,7 +21,7 @@ public LaunchRequestHandle(ILogger log) : base(log)
ILogger Log = log;

This comment has been minimized.

@BenjaminAbt

BenjaminAbt Mar 4, 2019

Contributor

this is a field, so please use the naming for fields. Please also add the protection level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.