Skip to content

(#176) HttpClientEnvelope + PlainHttpClient #189

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
Merged
merged 2 commits into from
Nov 27, 2018
Merged

(#176) HttpClientEnvelope + PlainHttpClient #189

merged 2 commits into from
Nov 27, 2018

Conversation

llorllale
Copy link
Contributor

This PR is for #176:

  • Added HttpClientEnvelope
  • Refactored existing http clients to extend HttpClientEnvelope
  • Added PlainHttpClient
  • Added ctor for RemoteDocker that defaults to the PlainHttpClient

@llorllale llorllale mentioned this pull request Nov 14, 2018
Closed
@0crat
Copy link
Collaborator

0crat commented Nov 14, 2018

Job #189 is now in scope, role is REV

@coveralls
Copy link

Pull Request Test Coverage Report for Build 324

  • 15 of 22 (68.18%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 82.669%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/com/amihaiemil/docker/RemoteDocker.java 0 2 0.0%
src/main/java/com/amihaiemil/docker/PlainHttpClient.java 0 5 0.0%
Totals Coverage Status
Change from base Build 321: 0.6%
Covered Lines: 415
Relevant Lines: 502
💛 - Coveralls

@coveralls
Copy link

coveralls commented Nov 14, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 328

  • 15 of 22 (68.18%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 82.669%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/com/amihaiemil/docker/RemoteDocker.java 0 2 0.0%
src/main/java/com/amihaiemil/docker/PlainHttpClient.java 0 5 0.0%
Totals Coverage Status
Change from base Build 321: 0.6%
Covered Lines: 415
Relevant Lines: 502
💛 - Coveralls

amihaiemil
amihaiemil reviewed Nov 18, 2018
View reviewed changes
Copy link
Owner

@amihaiemil amihaiemil left a comment

Choose a reason for hiding this comment

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

@llorllale Same here, just cosmetic comments. Btw, do we use SslHttpClient for anything anymore? :-?

src/main/java/com/amihaiemil/docker/HttpClientEnvelope.java
* HTTP client envelope.
*
* @author George Aristy (george.aristy@gmail.com)
* @since 0.0.4
Copy link
Owner

Choose a reason for hiding this comment

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

@llorllale @version $Id$ tag here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amihaiemil try using WriteTag to check for this

src/main/java/com/amihaiemil/docker/PlainHttpClient.java
* Plain HTTP (no TLS) client.
*
* @author George Aristy (george.aristy@gmail.com)
* @since 0.0.4
Copy link
Owner

Choose a reason for hiding this comment

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

@llorllale @version as well here

@llorllale
Copy link
Contributor Author

@amihaiemil added @version

do we use SslHttpClient for anything anymore?

RemoteDocker

@amihaiemil
Copy link
Owner

@rultor merge it

@rultor
Copy link
Collaborator

rultor commented Nov 27, 2018

@rultor merge it

@amihaiemil OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit f97da02 into amihaiemil:master Nov 27, 2018
@rultor
Copy link
Collaborator

rultor commented Nov 27, 2018

@rultor merge it

@amihaiemil Done! FYI, the full log is here (took me 2min)

@0crat
Copy link
Collaborator

0crat commented Nov 27, 2018

Job gh:amihaiemil/docker-java-api#189 is not assigned, can't get performer

@0crat
Copy link
Collaborator

0crat commented Nov 27, 2018

The job #189 is now out of scope

@amihaiemil amihaiemil mentioned this pull request Dec 6, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amihaiemil amihaiemil amihaiemil left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@llorllale @0crat @coveralls @amihaiemil @rultor