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.

Sign up for GitHub

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

Jump to bottom

Add windowsHide option to allow Windows popup from pm2 process file #3466

Merged
merged 7 commits into from
Feb 19, 2018
Merged

Add windowsHide option to allow Windows popup from pm2 process file #3466

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
387270d
Add windowsHide option so it can be set via process file, fixes #3425
natcl Feb 13, 2018
97fb295
Merge remote-tracking branch 'upstream/development' into development
natcl Feb 14, 2018
ab54f6a
Revert windowsHide to true for cluster mode, only needed in fork mode.
natcl Feb 14, 2018
031a668
Fix 'start pm2 inside pm2' test
natcl Feb 14, 2018
82375af
Revert to cleaner check
natcl Feb 15, 2018
767c36f
Check if windowsHide is bool
natcl Feb 16, 2018
f305d6d
Correct typo
natcl Feb 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/API/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,5 +213,9 @@
"instance_var": {
"type": "string",
"default" : "NODE_APP_INSTANCE"
},
"windowsHide": {
"type": "boolean",
"default" : true
}
}
2 changes: 1 addition & 1 deletion lib/God/ForkMode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ module.exports = function ForkMode(God) {
var cspr = spawn(command, args, {
env : pm2_env,
detached : true,
windowsHide: true,
windowsHide: pm2_env.windowsHide,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should do something like
windowsHide: pm2_env.windowsHide || true

Just in case variable is undefined, it's probably why tests are failing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

windowsHide: pm2_env.windowsHide || true wouldn't work though because false || true equals true so it will never be false (which is the goal).
I did try adding the following however to ensure that windowsHide is not undefined but it still failed the build test in travis (but succeeded on my machine):

var windowsHide = true;
if (pm2_env.windowsHide != undefined) {
    windowsHide = pm2_env.windowsHide
}

Copy link
Contributor

@wallet77 wallet77 Feb 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Check undefined is not enough I think, pm2_env.windowsHide can be null, or written as a string "false".
Maybe we should check if variable is a boolean, like so :
typeof(pm2_env.windowHide) === "boolean"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will test that now ! thanks !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fix worked but the tests failed for node4 although it doesn't seem related to those changes, is it possible it's a build error ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes sometimes tests failed on node4. I will rerun the job.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks !

cwd : pm2_env.pm_cwd || process.cwd(),
stdio : ['pipe', 'pipe', 'pipe', 'ipc'] //Same as fork() in node core
});
Expand Down