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

Factor out ScriptChildProcess class #198

Merged
merged 3 commits into from
May 9, 2022
Merged

Factor out ScriptChildProcess class #198

merged 3 commits into from
May 9, 2022

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented May 9, 2022

Factor out a new ScriptChildProcess class from the#executeCommandIfNeeded method. This class represents a spawned child process.

This will be useful for server mode, because it gives us a more discrete object that represents a running child process. The lifecycle of running processes will become more complex in server mode, especially in watch mode where we'll be persisting child processes potentially across different build graphs. In a follow up PR, it will gain a state property and a terminate method.

Also includes a small tweak to logging. We used to log the "Running" message before we tried to get a WorkerPool slot, which could give the impression that something was running that wasn't yet, when concurrency was constrained.

@aomarks
Copy link
Member Author

aomarks commented May 9, 2022

Sorry, just force pushed because I realized the incremental diff was actually more confusing than a full diff in this case. Ignore previous comment about looking at these one-by-one.

@aomarks aomarks merged commit 7cbce7f into main May 9, 2022
@aomarks aomarks deleted the spawn branch May 9, 2022 20:27
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