FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc, win: improve os.setPriority documentation by bzoz · Pull Request #22817 · nodejs/node · GitHub

/ node Public

doc, win: improve os.setPriority documentation - #22817

Closed
bzoz wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:bartosz-priority-test-fix
ConversationCommits (1)ChecksFiles changed
Closed

doc, win: improve os.setPriority documentation#22817
bzoz wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:bartosz-priority-test-fix

Conversation

bzoz commented Sep 12, 2018

Copy link
Copy Markdown
Contributor

Also fixes the test: #22799

See https://support.microsoft.com/en-us/help/110853/prb-can-t-increase-process-priority

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

nodejs-github-bot commented Sep 12, 2018

Copy link
Copy Markdown
Collaborator

@bzoz build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/859/pipeline

bzoz mentioned this pull request Sep 12, 2018
Closed
bzoz added a commit to JaneaSystems/libuv that referenced this pull request Sep 12, 2018
win, doc: improve uv_os_setpriority documentation
399fc69 
Ref: nodejs/node#22817
bzoz mentioned this pull request Sep 12, 2018
Closed
addaleax added doc Issues and PRs related to the documentations. windows Issues and PRs related to the Windows platform. os Issues and PRs related to the os subsystem. labels Sep 12, 2018
mscdex reviewed Sep 12, 2018
View reviewed changes
Comment thread test/parallel/test-os-process-priority.js
// for others it will be silently reduced to PRIORITY_HIGH
if (expected < PRIORITY_HIGH)
assert.strictEqual(priority, PRIORITY_HIGHEST);
assert.ok(priority === PRIORITY_HIGHEST || priority === PRIORITY_HIGH);

mscdex Sep 12, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Instead of doing this for all cases, can we make an explicit check for Windows and possibly for elevated user status before making this kind of assertion?

bzoz Sep 13, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is for Windows only (see line 114)

mscdex Sep 13, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I see, can we check for elevated status then? Perhaps using this method or something similar (without 3rd party utilities)?

danbev commented Sep 18, 2018

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/17275/

cjihrig pushed a commit to libuv/libuv that referenced this pull request Sep 18, 2018
win,doc: improve uv_os_setpriority() documentation
b721891 
Refs: nodejs/node#22817
PR-URL: #1985
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>

danbev commented Sep 19, 2018

Copy link
Copy Markdown
Contributor

Landed in 6e746f1.

danbev closed this Sep 19, 2018
danbev pushed a commit that referenced this pull request Sep 19, 2018
doc, win: improve os.setPriority documentation
6e746f1 
PR-URL: #22817
Fixes: #22799
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos mentioned this pull request Sep 19, 2018
Merged
targos pushed a commit that referenced this pull request Sep 19, 2018
doc, win: improve os.setPriority documentation
d4278a0 
PR-URL: #22817
Fixes: #22799
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Sep 20, 2018
doc, win: improve os.setPriority documentation
5da1f0c 
PR-URL: #22817
Fixes: #22799
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Sep 25, 2018
Open
Merged
cjihrig added a commit to cjihrig/libuv that referenced this pull request Sep 26, 2018
test: handle uv_os_setpriority() windows edge case
4bd0187 
Refs: nodejs/node#22817
Refs: libuv/help#64
PR-URL: libuv#2002
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes
cjihrig cjihrig approved these changes
tniessen tniessen approved these changes
+1 more reviewer
mscdex mscdex left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem. windows Issues and PRs related to the Windows platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL