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

win: add customization warning to tools script #24348

Closed

Conversation

joaocgreis
Copy link
Member

The script introduced in #22645 to install Visual Studio and Python on Windows should work reliably in most scenarios. But it can only go so far, if a user wants to customize the installation or has custom policies in place, the alternative installation methods should be used. The script is designed to be as simple as possible, because for other options and customization there are already detailed instructions that the users can follow.

This PR adds a warning about that.

cc @nodejs/platform-windows

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@joaocgreis joaocgreis added windows Issues and PRs related to the Windows platform. tools Issues and PRs related to the tools directory. dont-land-on-v6.x labels Nov 13, 2018
@nodejs-github-bot nodejs-github-bot added the install Issues and PRs related to the installers. label Nov 13, 2018
@refack
Copy link
Contributor

refack commented Nov 13, 2018

I'm not against clear text, but its effectiveness is limited, especially IMO when it's long.
Maybe we could use some GUI tricks like

Ohh there's a GUI to create WinForms - https://poshgui.com

https://theitbros.com/powershell-gui-for-scripts/
https://blogs.technet.microsoft.com/heyscriptingguy/2011/07/24/create-a-simple-graphical-interface-for-a-powershell-script/

@joaocgreis
Copy link
Member Author

Please upvote this comment to approve fast-tracking, it would be good if this could make it into v11.2.0.

cc @refack (as requested in collaborator guide)

CI: https://ci.nodejs.org/view/All/job/node-test-pull-request/18598/

@joaocgreis joaocgreis added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 14, 2018
@refack
Copy link
Contributor

refack commented Nov 14, 2018

Need another upvote. @nodejs/platform-windows @tniessen @nodejs/build-files

@refack
Copy link
Contributor

refack commented Nov 14, 2018

/CC @nodejs/documentation

@Trott
Copy link
Member

Trott commented Nov 14, 2018

@joaocgreis
Copy link
Member Author

Resumed CI green, landed in 3856d8a.

Thanks!

@joaocgreis joaocgreis closed this Nov 14, 2018
joaocgreis added a commit that referenced this pull request Nov 14, 2018
PR-URL: #24348
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
@joaocgreis
Copy link
Member Author

LTS: Depends on #24344, for now it looks like the revert is going forward so I'll add the don't land label here.

BridgeAR pushed a commit that referenced this pull request Nov 15, 2018
PR-URL: #24348
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
targos pushed a commit that referenced this pull request Nov 15, 2018
Notable changes:

* deps:
  * A new experimental HTTP parser (`llhttp`) is now supported.
    #24059
* timers:
  * Fixed an issue that could cause setTimeout to stop working as
    expected. #24322
* Windows
  * A crashing process will now show the names of stack frames if the
    node.pdb file is available.
    #23822
  * Continued effort to improve the installer's new stage that installs
    native build tools.
    #23987,
    #24348
  * child_process:
    * On Windows the `windowsHide` option default was restored to
      `false`. This means `detached` child processes and GUI apps will
      once again start in a new window.
      #24034
* Added new collaborators:
  * [oyyd](https://github.com/oyyd) - Ouyang Yadong.
    #24300
  * [psmarshall](https://github.com/psmarshall) - Peter Marshall.
    #24170
  * [shisama](https://github.com/shisama) - Masashi Hirano.
    #24136
targos pushed a commit that referenced this pull request Nov 15, 2018
Notable changes:

* deps:
  * A new experimental HTTP parser (`llhttp`) is now supported.
    #24059
* timers:
  * Fixed an issue that could cause setTimeout to stop working as
    expected. #24322
* Windows
  * A crashing process will now show the names of stack frames if the
    node.pdb file is available.
    #23822
  * Continued effort to improve the installer's new stage that installs
    native build tools.
    #23987,
    #24348
  * child_process:
    * On Windows the `windowsHide` option default was restored to
      `false`. This means `detached` child processes and GUI apps will
      once again start in a new window.
      #24034
* Added new collaborators:
  * [oyyd](https://github.com/oyyd) - Ouyang Yadong.
    #24300
  * [psmarshall](https://github.com/psmarshall) - Peter Marshall.
    #24170
  * [shisama](https://github.com/shisama) - Masashi Hirano.
    #24136

PR-URL: #24350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. install Issues and PRs related to the installers. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants