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

Remove posix runner and unsupported posix-spawn gem #19

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

elisuh
Copy link
Contributor

@elisuh elisuh commented Feb 9, 2023

This PR removes Terrapin's PosixRunner, which relies on the now unsupported posix-spawn gem. Improvements to Ruby (as of Ruby 3.0) means that posix-spawn is now less performant than native Ruby implementation. You can read more about this in this posix-spawn issue, specifically this comment.

We feel it's no longer necessary to provide posix support as there are no longer advantages over native. Since Ruby 2.x will no longer be supported soon, it's important that we maintain Terrapin to be compatible with the lowest supported version of Ruby.

Copy link
Contributor

@brian-penguin brian-penguin left a comment

Choose a reason for hiding this comment

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

NEAT!

Do you feel up to adding a CI matrix spec setup so we can see in the actions that it runs against all the versions we want to support? (doesn't have to be a part of this PR)

@brian-penguin
Copy link
Contributor

Oh dang looks like you are already on it #16

@elisuh
Copy link
Contributor Author

elisuh commented Feb 9, 2023

Ahhh that was all @joshuaclayton!!! But yea it's done, wanted to put it in a separate PR though since it's such a big change.

Copy link

@jutonz jutonz left a comment

Choose a reason for hiding this comment

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

Awesome, I love it! This is so much cleaner. I wonder if we could even just remove BackticksRunner at some point too, since Process.spawn should be available on all currently supported rubies.

I just had one small note about the Runners section of the readme, but other than that this looks great! And since this gem isn't 1.0 yet we don't have to worry about making a breaking change. Great work :)

@elisuh elisuh merged commit 7bfb3fe into main Feb 16, 2023
@elisuh elisuh deleted the ev-remove-posix-runner branch February 16, 2023 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants