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.

Sign up for GitHub

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

Jump to bottom

(bunq/sdk_java#55) add response id to request error #61

Merged
merged 15 commits into from
Jan 3, 2018
Merged

(bunq/sdk_java#55) add response id to request error #61

merged 15 commits into from
Jan 3, 2018

Conversation

OGKevin
Copy link
Contributor

@OGKevin OGKevin commented Dec 30, 2017

Closes #55

  • Tested

@OGKevin OGKevin added this to the 0.12.5 milestone Dec 30, 2017
@OGKevin OGKevin self-assigned this Dec 30, 2017
OGKevin
OGKevin commented Dec 30, 2017
View reviewed changes
Copy link
Contributor Author

@OGKevin OGKevin left a comment

Choose a reason for hiding this comment

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

Missing some new lines.

src/test/java/com/bunq/sdk/http/ErrorResponseTest.java Outdated
import static org.junit.Assert.assertNotNull;

public class ErrorResponseTest extends BunqSdkTestBase {
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enter

src/test/java/com/bunq/sdk/http/ErrorResponseTest.java Outdated
assertNotNull(caughtException);
assertNotNull(caughtException.getResponseId());
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enter

@bunq bunq deleted a comment Dec 30, 2017
@bunq bunq deleted a comment Dec 30, 2017
OGKevin
OGKevin commented Jan 2, 2018
View reviewed changes
Copy link
Contributor Author

@OGKevin OGKevin left a comment

Choose a reason for hiding this comment

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

No plural names. 💅

src/main/java/com/bunq/sdk/exception/ExceptionFactory.java Outdated
}
}

private static String concatenateMessages(List<String> messages) {
return String.join(GLUE_ERROR_MESSAGES, messages);
private static String concatenateMessages(List<String> messages, String responseId) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plural 🙅‍♂️

@OGKevin
Copy link
Contributor Author

OGKevin commented Jan 2, 2018

@patrickdw1991 all yours please 👀

@bunq bunq deleted a comment Jan 2, 2018
patrickdw1991
patrickdw1991 previously approved these changes Jan 2, 2018
View reviewed changes
@OGKevin OGKevin force-pushed the feature/bunq/sdk_java#55-add_response_id_to_request_error branch from 21ef1eb to 7bbc4c6 Compare January 2, 2018 09:39
@OGKevin
Copy link
Contributor Author

OGKevin commented Jan 2, 2018

@andrederoos all yours please 👀

@bunq bunq deleted a comment Jan 2, 2018
andrederoos
andrederoos requested changes Jan 2, 2018
View reviewed changes
src/main/java/com/bunq/sdk/http/ApiClient.java Outdated
String responseBody) {
List<String> errorDescriptions = new ArrayList<>();
String responseBody, String responseId) {
List<String> AllErrorDescription = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

why the capital?

src/main/java/com/bunq/sdk/http/ApiClient.java Outdated
return headerMap.get(HEADER_RESPONSE_ID_LOWER_CASE);
} else if (headerMap.containsKey(HEADER_RESPONSE_ID_UPPER_CASE)) {
return headerMap.get(HEADER_RESPONSE_ID_UPPER_CASE);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing the else...

@bunq bunq deleted a comment Jan 2, 2018
@bunq bunq deleted a comment Jan 2, 2018
@OGKevin OGKevin force-pushed the feature/bunq/sdk_java#55-add_response_id_to_request_error branch from d5b6270 to 1d00e46 Compare January 3, 2018 08:14
@bunq bunq deleted a comment Jan 3, 2018
@bunq bunq deleted a comment Jan 3, 2018
@andrederoos andrederoos merged commit faf05ff into develop Jan 3, 2018
@andrederoos andrederoos deleted the feature/bunq/sdk_java#55-add_response_id_to_request_error branch January 3, 2018 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrederoos andrederoos andrederoos approved these changes

@patrickdw1991 patrickdw1991 patrickdw1991 left review comments

Assignees

@andrederoos andrederoos

@OGKevin OGKevin

Labels
Can be merged Reviewed by André
Projects
None yet
Milestone
0.12.5
Development

Successfully merging this pull request may close these issues.
3 participants
@OGKevin @patrickdw1991 @andrederoos