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: typo: interal->internal. #5849

Closed
wants to merge 1 commit into from
Closed

doc: typo: interal->internal. #5849

wants to merge 1 commit into from

Conversation

kosak
Copy link
Contributor

@kosak kosak commented Mar 22, 2016

Pull Request check-list

Please make sure to review and check all of these items:

  • [N/A] Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • [N/A] If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • [N/A] Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

Please provide affected core subsystem(s) (like buffer, cluster, crypto, etc)

Description of change

Fixes a typo in the API documentation (api/events)

@jasnell
Copy link
Member

jasnell commented Mar 22, 2016

LGTM (update: for the events.markdown fix... I had missed that this touched on a dep file also)

@mscdex mscdex added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Mar 22, 2016
@@ -24,7 +24,7 @@ v0.1.0 -- 2014.04.29
* Remove comparator support (as it was removed from spec)
* Provide @@toStringTag symbol, ad @@iterator symbols on iterators
* Update to use latest dependencies versions
* Improve interals
* Improve internals
Copy link
Member

Choose a reason for hiding this comment

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

This is an external dependency so we shouldn't fix it here, but rather submit a PR upstream to the es6-set project.

@Trott
Copy link
Member

Trott commented Mar 22, 2016

Changes to events.markdown look good to me. Change to es6-sets needs to go to that project. It's an external dependency.

@kosak
Copy link
Contributor Author

kosak commented Mar 22, 2016

Gotcha. Sorry about that. I've amended the commit to exclude the es6-sets file.

benjamingr pushed a commit that referenced this pull request Mar 22, 2016
Fixes a copy typo in the events.md docs.

PR-URL: #5849
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@benjamingr
Copy link
Member

Landed in e301f97 thanks!

@benjamingr benjamingr closed this Mar 22, 2016
evanlucas pushed a commit that referenced this pull request Mar 30, 2016
Fixes a copy typo in the events.md docs.

PR-URL: #5849
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 30, 2016
Fixes a copy typo in the events.md docs.

PR-URL: #5849
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 30, 2016
Fixes a copy typo in the events.md docs.

PR-URL: #5849
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request Mar 31, 2016
Fixes a copy typo in the events.md docs.

PR-URL: #5849
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
sam-github pushed a commit to sam-github/node that referenced this pull request Dec 14, 2016
The MSI install scope was set to the WiX default, which is per-user.
However, with UAC, it could not be installed by a standard user because
InstallPrivileges is elevated by default, hence the install scope
should be set to per-machine. Furthermore, the default install path is
a per-machine location and setting the system path requires
administrator privileges.

By changing the InstallScope to perMachine, Start Menu shortcuts are
placed in ProgramData and not the installing user's AppData folder,
making the shortcuts available to other users. This also fixes the
installation when AppData is a network folder.

The custom action is necessary to allow upgrades. Since a per-machine
MSI cannot upgrade an application installed per-user, the custom action
checks if there is going to be an upgrade to a previous version
installed per-user and sets the installation as per-user to allow
upgrading. Hence, the advantages of installing per-machine will only
apply in fresh installations.

Fixes nodejs#5849
Fixes nodejs#7629

PR-URL: nodejs/node-v0.x-archive#25640
Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
joaocgreis added a commit to JaneaSystems/node that referenced this pull request Aug 24, 2017
The MSI install scope was set to the WiX default, which is per-user.
However, with UAC, it could not be installed by a standard user because
InstallPrivileges is elevated by default, hence the install scope
should be set to per-machine. Furthermore, the default install path is
a per-machine location and setting the system path requires
administrator privileges.

By changing the InstallScope to perMachine, Start Menu shortcuts are
placed in ProgramData and not the installing user's AppData folder,
making the shortcuts available to other users. This also fixes the
installation when AppData is a network folder.

The custom action is necessary to allow upgrades. Since a per-machine
MSI cannot upgrade an application installed per-user, the custom action
checks if there is going to be an upgrade to a previous version
installed per-user and sets the installation as per-user to allow
upgrading. Hence, the advantages of installing per-machine will only
apply in fresh installations.

Fixes nodejs#5849
Fixes nodejs#7629
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants