Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

silent setup #264

Merged
merged 2 commits into from
Aug 2, 2018
Merged

silent setup #264

merged 2 commits into from
Aug 2, 2018

Conversation

fredericrous
Copy link
Contributor

  • silent install without prompt as it's expected when we install nvm-windows from a script
  • silent uninstall
  • fix a warning (Warning: Line 80, Column 10: [Hint] Variable 'Result' never used)

- silent install without prompt as it's expected when we install nvm-windows from a script
- silent uninstall
- fix a warning (Warning: Line 80, Column 10: [Hint] Variable 'Result' never used)
@fredericrous
Copy link
Contributor Author

fredericrous commented May 4, 2017

fix #210 (and #17 ?)

- silent install without prompt as it's expected when we install nvm-windows from a script
- silent uninstall
- fix a warning (Warning: Line 80, Column 10: [Hint] Variable 'Result' never used)
@fredericrous
Copy link
Contributor Author

on my first commit 2f27f4f I used WizardSilent and UninstallSilent variables to detect if nvm setup is in silent mode in order to suppress the message boxes. In the second commit 40d9063 I replaced MsgBox function with SuppressibleMsgBox which is better suited I believe. Indeed the code is simpler and it follows better the inno setup philosophy. use nvm-setup.exe /SILENT /SUPPRESSMSGBOXES to execute the setup without being prompted with message boxes

@fredericrous
Copy link
Contributor Author

fredericrous commented May 21, 2017

Hi @coreybutler I believe this PR is easy to test & review, what do you think? would you merge it?

@coreybutler
Copy link
Owner

I'm sorry for the delayed response... I had a very intense client project over the last few weeks.

At first glance, I am not opposed to merging this, but I would like to run a few tests myself. I really appreciate what you've done... not many people dive into the installer. I'll review this in more detail as soon as I can.

@Inveracity
Copy link

This would be lovely to have, commenting to bring attention to this 🏓

@fredericrous
Copy link
Contributor Author

Yup, I was tired to wait I bought a mac. I'm only half joking, I'm on mac now.

Copy link
Owner

@coreybutler coreybutler left a comment

Choose a reason for hiding this comment

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

LGTM

@coreybutler coreybutler merged commit 2065cea into coreybutler:master Aug 2, 2018
@fredericrous fredericrous deleted the patch-1 branch August 3, 2018 00:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants