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

Allow datamill client to perform PATCH requests #35

Merged
merged 5 commits into from
May 16, 2016

Conversation

israelcolomer
Copy link
Collaborator

Reimplemented client request core method to rely on httpclient rather than JDK UrlConnection, as the latter doesn't allow certain Http methods (i.e. PATCH).
Adapted tests to new implementation.

Reimplemented client request core method to rely on httpclient rather than JDK UrlConnection, as the latter doesn't allow certain Http methods (i.e. PATCH).
Adapted tests to new implementation.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 70.625% when pulling 1b0fa95 on issue-34-patch-requests into cc361a3 on master.

URIBuilder uriBuilder = new URIBuilder();


int firstSlash = uri.indexOf('/');
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure I understand why you are doing all the string manipulation here - I think just create a URI straight from the string and let the URI implementations deal with it? Also, "theURI" and "anURI" are not good names

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dunno what I was thinking sorry..late hours programming 😁

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 70.52% when pulling bf1bf1a on issue-34-patch-requests into cc361a3 on master.

Applied more comments, refactored and cleanup.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 70.625% when pulling 3474b0f on issue-34-patch-requests into cc361a3 on master.

Mock execute method in client tests so they run faster and don't send requests (which arent checked anyway)
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 70.337% when pulling 96e0f30 on issue-34-patch-requests into cc361a3 on master.

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.

None yet

3 participants