Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

Remove typhoeus from oc-pushy-pedant #155

Merged
merged 2 commits into from
Nov 18, 2016
Merged

Remove typhoeus from oc-pushy-pedant #155

merged 2 commits into from
Nov 18, 2016

Conversation

stevendanna
Copy link
Contributor

typhoeus depends on libcurl at runtime. We would like to remove curl
from the build. It appears that typhoeus is actually unused in the code
so we can drop this dependency.

Signed-off-by: Steven Danna [email protected]

rhass
rhass previously requested changes Nov 18, 2016
Copy link
Contributor

@rhass rhass left a comment

Choose a reason for hiding this comment

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

@stevendanna
Copy link
Contributor Author

@rhass Ah, forgot to move that class. I believe that the tests are only really using EventStreamOld and EventStream isn't instantiated anywhere. If we'd prefer to keep the "EventStream" class then we'll need to re-add curl.

typhoeus depends on libcurl at runtime. We would like to remove curl
from the build. It appears that typhoeus is actually unused in the code
so we can drop this dependency.

The EventStream class that uses Typhoeus was dead code. This class has
been removed and the EventStreamOld class has been renamed to
EventStream.

Signed-off-by: Steven Danna <[email protected]>
@stevendanna
Copy link
Contributor Author

@ksubrama
Copy link

A lot of our curl based code seems to be old code to avoid the SSL cert issue we ran into with Ruby 2 years ago. Those are no longer an issue with a modern ruby and our omnibus builds. So +1 on dropping that class.

@smith smith dismissed rhass’s stale review November 18, 2016 17:00

Changes have been made to address this.

@smith smith merged commit 6d9d98b into master Nov 18, 2016
@smith smith deleted the ssd/no-libcurl branch November 18, 2016 17:01
@smith smith removed the in progress label Nov 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants