Skip to content

Conversation

@WyriHaximus
Copy link
Member

Mutually exclusive with #154

This came up in a discussion after #154 was filed and because doing this by default will be provide a better BC.

Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

This looks like a sane default behavior to me as it also fixes issues with STDIO-streams exposing the underlying FD is a file is piped etc..

I would vote to apply this as default behavior and remove the config parameter altogether as per #154 until we find a valid use case where an explicit config makes more sense?

@WyriHaximus
Copy link
Member Author

Makes sense, I'll update the PR 👍

@WyriHaximus
Copy link
Member Author

Done

@clue clue added the BC break label Mar 13, 2018
@clue clue changed the title ExtEventLoop make FEATURE_FDS enabled by default ExtEventLoop: Remove optional EventConfig and make FEATURE_FDS enabled by default Mar 13, 2018
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

Awesome, LGTM! 👍

@clue clue changed the title ExtEventLoop: Remove optional EventConfig and make FEATURE_FDS enabled by default ExtEventLoop: Remove optional EventBaseConfig and make FEATURE_FDS enabled by default Mar 13, 2018
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.

3 participants