yegor256 / cactoos-http

Object-Oriented HTTP Client
MIT License
37 stars 28 forks source link

(#98) Getting rid of the `BlockingSocketServer` at all #101

Closed iakunin closed 5 years ago

iakunin commented 5 years ago

This is for https://github.com/yegor256/cactoos-http/issues/98

@victornoel @paulodamaso I've found a solution that could help up to get rid of the BlockingSocketServer: https://stackoverflow.com/a/904609/3456163

Let's see if it works in rultor-environment.

0crat commented 5 years ago

Job #101 is now in scope, role is REV

codecov-io commented 5 years ago

Codecov Report

Merging #101 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #101   +/-   ##
=========================================
  Coverage     95.19%   95.19%           
  Complexity       76       76           
=========================================
  Files            20       20           
  Lines           208      208           
  Branches         13       13           
=========================================
  Hits            198      198           
  Misses            7        7           
  Partials          3        3

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 74259a4...b949857. Read the comment docs.

0crat commented 5 years ago

This pull request #101 is assigned to @victornoel/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job

iakunin commented 5 years ago

@iakunin great idea, I'm glad we found a simpler solution. I'm just worried about making this more explicit. Do we see any idea how to better document why this work? Maybe let's just leave the SO url as a comment to explain why we use this to test timeout?

@victornoel and I'm happy to suggest such simpler solution. I agree with you about the comment. I left one about artificial timeout.

iakunin commented 5 years ago

@iakunin one two small changes needed

I've done it. That's not a problem :)

victornoel commented 5 years ago

@iakunin thanks! @paulodamaso it's good to merge

iakunin commented 5 years ago

@paulodamaso @victornoel let's have a look one more time, please

paulodamaso commented 5 years ago

@iakunin @victornoel Great work on this one guys

paulodamaso commented 5 years ago

@rultor merge

rultor commented 5 years ago

@rultor merge

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

rultor commented 5 years ago

@rultor merge

@paulodamaso Done! FYI, the full log is here (took me 11min)

0crat commented 5 years ago

@sereshqua/z please review this job completed by @victornoel/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

0crat commented 5 years ago

The job #101 is now out of scope

0crat commented 5 years ago

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @paulodamaso/z

sereshqua commented 5 years ago

@iakunin please make sure you start all your comments with the name of the user they are addressed to, see

@victornoel issue from yesterday repeats again :)

iakunin commented 5 years ago

@sereshqua you’re right. My bad.

sereshqua commented 5 years ago

@0crat quality acceptable

0crat commented 5 years ago

Order was finished, quality is "acceptable": +15 point(s) just awarded to @victornoel/z

0crat commented 5 years ago

Quality review completed: +4 point(s) just awarded to @sereshqua/z

iakunin commented 5 years ago

@iakunin @victornoel Great work on this one guys

@paulodamaso @victornoel it's pleasure to work with you, guys :)