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

#2182 Get rid of annoying popups in Windows 10 #3400

Merged
merged 2 commits into from
Jan 26, 2018
Merged

#2182 Get rid of annoying popups in Windows 10 #3400

merged 2 commits into from
Jan 26, 2018

Conversation

toddwong
Copy link
Contributor

@toddwong toddwong commented Jan 12, 2018

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #2182
License MIT

Works with node v9.4.0 and above.

@CLAassistant
Copy link

CLAassistant commented Jan 12, 2018

CLA assistant check
All committers have signed the CLA.

@@ -93,7 +93,6 @@ module.exports = function ForkMode(God) {
try {
var cspr = spawn(command, args, {
env : pm2_env,
detached : true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you really need to remove detached here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@soyuka I can't make the windowsHide option works with detached: true. Do we actually needs detached to be true here? I maybe mistook something here, but child processes in cluster mode are attached to the daemon process I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@soyuka I tried to reproduce the problem (detached:true with 'windowsHide:true`) to find the real cause, but found something unexpected. I think the console windows may be not caused by this spawn call at all, but something else. I'll do some digging.

@toddwong
Copy link
Contributor Author

@soyuka I've found the root cause of the popping up console window while start process in fork mode. It's not the spawn call in lib/God/ForkMode.js, but pmx.init in lib/ProcessContainerFork.js. I've submit a PR to the pmx project, and undone the detached: true deletion.

@soyuka
Copy link
Collaborator

soyuka commented Jan 13, 2018

Great work @toddwong thanks!

@vmarchaud vmarchaud self-requested a review January 15, 2018 12:50
@vmarchaud
Copy link
Contributor

Tests failed are once again not linked to your PR, i will merge it when i finished with pmx :)

@toddwong
Copy link
Contributor Author

Thanks, @vmarchaud !

@soyuka
Copy link
Collaborator

soyuka commented Jan 26, 2018

ping @wallet77

@wallet77
Copy link
Contributor

Yes I restarted builds but we have troubles with Travis.
But I will approve this one soon ;)
It will be in the next release !

@wallet77 wallet77 self-requested a review January 26, 2018 14:57
@wallet77 wallet77 merged commit f65e879 into Unitech:development Jan 26, 2018
inerc pushed a commit to inerc/pm2 that referenced this pull request Feb 11, 2020
@linonetwo
Copy link

Will this work in fork mode?

Cluster mode won't run ps1 file in windows. And fork mode still pop up cmd window.

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.

6 participants