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

added text regarding alternate user elevation. #24343

Closed
wants to merge 5 commits into from

Conversation

MPagel
Copy link

@MPagel MPagel commented Nov 13, 2018

Added instructions for alternate user elevation for UAC system.

Struck "If you prematurely stop the process, UAC will need to be re-
enabled manually." When UAC is disabled the user account ironically
may not have access to re-enable it, so manual re-enabling is a non-
trivial matter. Unless instructions are given, this "help" will just
lead to confusion.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Added instructions for alternate user elevation for UAC system.

Struck "If you prematurely stop the process, UAC will need to be re-enabled manually." When UAC is disabled the user account ironically may not have access to re-enable it, so manual re-enabling is a non-trivial matter. Unless instructions are given, this "help" will just lead to confusion.
@nodejs-github-bot nodejs-github-bot added 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. labels Nov 13, 2018
@MPagel
Copy link
Author

MPagel commented Nov 13, 2018

cross-ref pull #23987

@vsemozhetbyt
Copy link
Contributor

Thank you. Just some nits)

vsemozhetbyt and others added 2 commits November 13, 2018 12:26
+1

Co-Authored-By: MPagel <MPagel@users.noreply.github.com>
@MPagel
Copy link
Author

MPagel commented Nov 13, 2018

thanks. I THINK both of your edits are now integrated.

@vsemozhetbyt
Copy link
Contributor

сс @nodejs/platform-windows

<=72 characters echo'd per line to satisfy travis

includes some minor language changes
@MPagel MPagel changed the title added text re: alternate user elevation. added text regarding alternate user elevation. Nov 13, 2018
Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

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

@MPagel thanks for opening this PR and trying to make this feature better! I'm sorry that this did not work as you expected.

I don't think the way forward here is to add more instructions and workarounds, but to warn the users that this is designed to be simple, not customizable. I opened a PR to address this: #24348

I wouldn't be surprised if there are more corner cases like this. Using Boxstarter and Chocolatey gives us a great base, they install dependencies and solve many issues that could make the installation fail. However, they can only go so far. If a user wants to customize the installation or has custom policies in place like what you described in #23987 (comment), the installers should be used directly.

@Trott
Copy link
Member

Trott commented Nov 15, 2018

It seems like perhaps this should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

@Trott Trott closed this Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants