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

os.FindProcess, process.Release #4218

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

leongross
Copy link

@leongross leongross commented Apr 5, 2024

This implementation of os.FindProcess provides minimal functionality while being aligned to the process design pattern of tinygo.

@leongross leongross changed the title os.FindProcess strub os.FindProcess stub Apr 5, 2024
@leongross leongross force-pushed the os/FindProcess branch 2 times, most recently from b27bafa to dd061d8 Compare April 6, 2024 07:43
@leongross leongross marked this pull request as ready for review April 24, 2024 12:13
Signed-off-by: leongross <[email protected]>
@leongross leongross force-pushed the os/FindProcess branch 2 times, most recently from 91b9c03 to 2f2caa6 Compare May 7, 2024 15:08
Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Comment on lines 14 to 15
// NOTE: For now, we only test the Linux case since only exec_posix.go is currently the only implementation.
if runtime.GOOS == "linux" {
Copy link
Member

Choose a reason for hiding this comment

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

exec_posix.go does in fact include Windows, and because it's all no-ops anyway I think it would be fine to test this on any platform (including Windows).

Copy link
Author

Choose a reason for hiding this comment

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

that makes sense, I will remove it

@leongross leongross changed the title os.FindProcess stub os.FindProcess, process.Release May 17, 2024
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.

None yet

2 participants