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

Add option to spawn processes as siblings #3012

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

jprendes
Copy link
Contributor

@jprendes jprendes commented Dec 5, 2024

This PR adds the option to spawn processes as siblings of the calling processes instead of spawning them as children.
See #3011 for more context.

@utam0k
Copy link
Member

utam0k commented Dec 6, 2024

Is there a good way to test this behavior?

@utam0k utam0k self-requested a review December 6, 2024 06:56
@jprendes
Copy link
Contributor Author

jprendes commented Dec 6, 2024

I think we can create an integration type of test, and check the ppid of the init process. I'll give that a go later today.

@jprendes
Copy link
Contributor Author

jprendes commented Dec 6, 2024

@utam0k, I´ve added two integration tests, one to check that it can run as a child, and one to check that it can run as a sibling.

@YJDoc2, I don't think this is a breaking change, as it´s adding a new method, not removing or changing an existing one.

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Dec 7, 2024

@YJDoc2, I don't think this is a breaking change, as it´s adding a new method, not removing or changing an existing one.

My apologies. I saw that the builder was a pub struct, and we were updating the fields, so it was breaking. But we actually don't expose the fields, only the builder methods, so this should not be a breaking change. Have removed the label.

Copy link
Collaborator

@YJDoc2 YJDoc2 left a comment

Choose a reason for hiding this comment

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

looks good to me 👍
Maybe wait for utam0k 's reply, but approving from my side.

@utam0k
Copy link
Member

utam0k commented Dec 9, 2024

Thanks!

@utam0k utam0k merged commit ae3f11b into youki-dev:main Dec 9, 2024
28 checks passed
@github-actions github-actions bot mentioned this pull request Dec 9, 2024
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