Skip to content

/ http4s

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

Jump to bottom

fix hang on master #2050

Merged
merged 2 commits into from Sep 5, 2018
Merged

fix hang on master #2050

merged 2 commits into from Sep 5, 2018

Conversation

@jmcardon
Copy link
Member

@jmcardon jmcardon commented Sep 5, 2018

No description provided.

@jmcardon
fix hang on master
Loading status checks…
1d38297
@rossabaker
Copy link
Member

@rossabaker rossabaker commented Sep 5, 2018

I'd rather replace Http4sSpec.TestExecutionContext with global. There's no reason to have two.
@jmcardon jmcardon force-pushed the jmcardon:fix-ws-hang branch from 607491a to 6c20654 Sep 5, 2018
@rossabaker
rossabaker reviewed Sep 5, 2018
View changes
Copy link
Member

@rossabaker rossabaker left a comment

👍 on green. These aren't leaking like I thought they did. (The cats-effect scheduler still does.)
@jmcardon jmcardon dismissed rossabaker’s stale review Sep 5, 2018

Tests are erroring out

@jmcardon
Copy link
Member Author

@jmcardon jmcardon commented Sep 5, 2018

This change is a tad bigger than I thought it was, one test is erroring out.
@jmcardon
modify testEc
Loading status checks…
75be580
@jmcardon jmcardon force-pushed the jmcardon:fix-ws-hang branch from 6c20654 to 75be580 Sep 5, 2018
@jmcardon
Copy link
Member Author

@jmcardon jmcardon commented Sep 5, 2018

I changed the pool to use the global pool purely for my test which abuses concurrency.
@rossabaker
rossabaker approved these changes Sep 5, 2018
View changes
Copy link
Member

@rossabaker rossabaker left a comment

It stops the bleeding. We can clean up the test pools later.
@rossabaker rossabaker merged commit f5416cd into http4s:master Sep 5, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rossabaker rossabaker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@jmcardon @rossabaker