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

doc: Update README.md #2822

Merged
merged 3 commits into from Apr 10, 2023
Merged

doc: Update README.md #2822

merged 3 commits into from Apr 10, 2023

Conversation

beliaev-maksim
Copy link
Contributor

Update installation instruction

README.md Outdated Show resolved Hide resolved
Co-authored-by: Christian Clauss <cclauss@me.com>
README.md Show resolved Hide resolved
@gengjiawen gengjiawen changed the title Update README.md doc: Update README.md Apr 10, 2023
@gengjiawen gengjiawen merged commit c7927e2 into nodejs:main Apr 10, 2023
31 checks passed
@gengjiawen
Copy link
Member

thx for the contribution

@beliaev-maksim beliaev-maksim deleted the patch-1 branch April 11, 2023 09:56
@karlhorky
Copy link
Contributor

karlhorky commented Aug 23, 2023

@gengjiawen in comment 1161506873: Can you delete this line ? I think for most user, this is not necessary.

@gengjiawen some of our students with Windows machines have problems with running this npm config set msvs_version 2017 command as part of our System Setup guide for Windows, so it's interesting to us that you removed it with the explanation that "it's not necessary for most users".

Their problems look like upleveled/system-setup#31:

PS C:\Windows\system32> npm config set msvs_version 2017
npm ERR! `msvs_version` is not a valid npm option

npm ERR! A complete log of this run can be found in: C:\Users\khayo\AppData\Local\npm-cache\_logs\2023-08-22T12_12_13_960Z-debug-0.log

Can you expand on the explanation why it's not necessary for most users? Which users would it be necessary for?

Our Windows System Setup Guide suggests to install the Chocolatey packages visualstudio2017-workload-vctools and python and then configure msvs_version, which leads to this failure.

If you have any suggestions or remarks, we're happy to also do the setup a different way - but it should be able to be performed via the command line, no GUI tools or downloads from websites.

@gengjiawen
Copy link
Member

gengjiawen commented Aug 23, 2023

Can you expand on the explanation why it's not necessary for most users?

Gyp can automatically choose msvs version.

Which users would it be necessary for?

You have a project only can compile in vs2017, but tbh MS don't maintain old visual studio.

My general advice: Install visual studio and Python on windows is only needed for node-gyp.

@karlhorky
Copy link
Contributor

karlhorky commented Aug 23, 2023

Ok great, then if I'm understanding correctly, @ProchaLu will try to upgrade to visualstudio2022-workload-vctools ("Visual C++ build tools workload for Visual Studio 2022 Build Tools 1.0.0") and remove the npm config set msvs_version ... command to see if this works, thanks!

kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Aug 29, 2023
Ref: nodejs/node-gyp#2822
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Aug 30, 2023
remove `msvs_version` npm config on win32
Ref: nodejs/node-gyp#2822

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Sep 1, 2023
remove `msvs_version` npm config on win32
Ref: nodejs/node-gyp#2822

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Sep 26, 2023
remove `msvs_version` npm config on win32
Ref: nodejs/node-gyp#2822

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
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.

None yet

4 participants