Skip to content

Conversation

@Aleffio
Copy link
Contributor

@Aleffio Aleffio commented Oct 1, 2018

No description provided.

@coveralls
Copy link

coveralls commented Oct 1, 2018

Coverage Status

Coverage increased (+0.1%) to 26.52% when pulling d6f786c on PW-647 into f159322 on develop.

this.config.setConnectionTimeoutMillis(connectionTimeoutMillis);
}

public Client(String username, String password, Environment environment, String applicationName, String liveEndpointUrlPrefix) {
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove it as applicationName will be removed and not needed after we implement applicationInformation

this.config.setCheckoutEndpoint(CHECKOUT_ENDPOINT_LIVE);
} else {
// throw exception
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if you use setEnvironment(environment) this is not setting any live endpoints any more ? would it not be better to just call setEnvironment(environment, null) and handle the null value as we already do ?

this.config.setEndpoint(ENDPOINT_LIVE);
this.config.setCheckoutEndpoint(null); // not supported please specify unique identifier
}

Copy link
Contributor

Choose a reason for hiding this comment

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

remove empy line.

if (client.getConfig().getCheckoutEndpoint() == null || client.getConfig().getCheckoutEndpoint().isEmpty()) {
String message = "Please specify the unique identifier when setting the environment";
throw new IllegalArgumentException(message);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicate code, let's find a better spot to reduct duplication

this.config.setCheckoutEndpoint(ENDPOINT_PROTOCOL + liveEndpointUrlPrefix + CHECKOUT_ENDPOINT_LIVE_SUFFIX);
} else {
this.config.setEndpoint(ENDPOINT_LIVE);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe best to setCheckoutEndpoint to null so if you first do a setEnvironment(TEST) and then setEnvironment(LIVE) you don't use the test endpoint and our exception can be triggered. Now it will ignore it as it has a value.

@Aleffio Aleffio merged commit 3f8d727 into develop Oct 4, 2018
@rkewlani rkewlani deleted the PW-647 branch September 24, 2020 19:11
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

Successfully merging this pull request may close these issues.

7 participants