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

Test Mac OS X on Travis #112

Merged
merged 2 commits into from
Oct 10, 2017
Merged

Test Mac OS X on Travis #112

merged 2 commits into from
Oct 10, 2017

Conversation

clue
Copy link
Member

@clue clue commented Oct 9, 2017

Squashed by @clue, originally from
reactphp/socket#124

Resolves / closes #111

Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

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

LGTM well done @andig and @clue 👍

@clue
Copy link
Member Author

clue commented Oct 9, 2017

I can't vote on my own PR, but I'm tempted to suggest we reject this PR. While I do think testing against Mac OS X adds some value, I'm not sure it's worth the wait, as Travis currently takes around 15-20min to start a single Mac OS X build. I would suggest closing this PR and let me cherry-pick the actual test fix as a separate PR?

What do you think about this?

@WyriHaximus
Copy link
Member

@clue actually I don't mind that wait since we generally don't respond within the time it takes Travis to run that OS X build

Copy link
Member

@jsor jsor left a comment

Choose a reason for hiding this comment

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

I don't mind the build time either. If it makes sense, i'm fine with enabling tests on OS X.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failures on OSX
4 participants