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

V11.5.0 proposal #25102

Merged
merged 89 commits into from
Dec 18, 2018
Merged

V11.5.0 proposal #25102

merged 89 commits into from
Dec 18, 2018

Conversation

BethGriggs
Copy link
Member

@BethGriggs BethGriggs commented Dec 18, 2018

2018-12-18, Version 11.5.0 (Current), @BethGriggs

Notable Changes

  • tls:
    • support "BEGIN TRUSTED CERTIFICATE" for ca: (Sam Roberts) #24733
  • tools:
    • update ESLint to 5.10.0 (cjihrig) #24903
  • util:
    • add inspection getter option (Ruben Bridgewater) #24852

Commits

cclauss and others added 30 commits December 7, 2018 16:29
PR-URL: #24801
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #24793
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This commit reduces shared state by introducing scopes and
block scoped variables. It also makes the monkey patching used
by the test more robust.

PR-URL: #24834
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This commit replaces a call to Number.isFinite() with a
cheaper typeof check. The subsequent range checks ensure that
the checked value is finite.

PR-URL: #24836
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.

PR-URL: #24840
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Currently the following compiler warnings are generated:
../src/node_util.cc:59:16:
warning: unused variable 'env' [-Wunused-variable]
  Environment* env = Environment::GetCurrent(args);
               ^
../src/node_util.cc:78:16:
warning: unused variable 'env' [-Wunused-variable]
  Environment* env = Environment::GetCurrent(args);
               ^
2 warnings generated.

This commit removes the two unused variables.

PR-URL: #24820
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
This commit removes the finalized_ member from the Hash class as it does
not seem to be used in any valuable way. Commit c75f87c ("crypto:
refactor the crypto module") removed the check where it was previously
used.

PR-URL: #24822
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Revise the Waiting for Approvals section of the Collaborator Guide. Keep
sentences short and clear. Split long paragraphs into shorter
paragraphs.

PR-URL: #24845
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
PR-URL: #24799
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
PR-URL: #24791
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
PR-URL: #24797
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #24798
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
Update the `LICENSE` file by running `tools/license-builder.sh`.

PR-URL: #24898
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
PR-URL: #24779
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
* Check for custom Node.js code rather than constructor in
  assert.throws().
* Use arrow functions consistently.

PR-URL: #24859
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: #24844
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Instead of using magic values for the server timeout in
test-http2-session-timeout, measure the duration of the first request
(which should be longer than subsequent requests) and use that value.

Fixes: #20628

PR-URL: #24877
Fixes: #20628
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
A fix to test-http2-session-timeout makes it sufficiently robust that it
can be moved to the parallel directory.

PR-URL: #24877
Fixes: #20628
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: #24403

PR-URL: #24907
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
For new Agent() signature in http doc, list the supported options in
socket.connect().

Refs: #24098

PR-URL: #24846
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Instead of using a shared pointer of the entire debug option set,
pass the parsed debug option to inspector classes by value because
they are set once the CLI argument parsing is done. Add another shared
pointer to HostPort being used by the inspector server, which is copied
from the one in the debug options initially. The port of the shared
HostPort is 9229 by default and can be specified as 0 initially but
will be set to the actual port of the server once it starts listening.

This makes the shared state clearer and makes it possible to use
`require('internal/options')` in JS land to query the CLI options
instead of using `process._breakFirstLine` and other underscored
properties of `process` since we are now certain that these
values should not be altered once the parsing is done and can be
passed around in copies without locks.

PR-URL: #24772
Reviewed-By: Anna Henningsen <[email protected]>
Compiler version tuples should be numeric for tuple comparisons
to work.

Also correct check for AIX where the minimum supported GCC is 6.3.0

PR-URL: #24879
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
In case of an error where we only care about a cleaned up stack
trace it is cheaper to reset the stack trace limit for the error
that is created. That way the stack frames do not have to be
computed twice.

PR-URL: #24747
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
PR-URL: #24863
Refs: #22101
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Update "Testing and CI" in the Collaborator Guide. Remove redundant
material. Shorten sentences. Remove incorrect material. (Specifically,
we don't require test cases to be included in all pull requests that
modify exectuable code. For example, if code is refactored, then passing
existing tests is sufficient.)

PR-URL: #24884
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: #24882
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
The label should be applied early on. Otherwise there is little
benefit using this label at all.

PR-URL: #24893
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Update ESLint to 5.10.0.

PR-URL: #24903
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. v11.x labels Dec 18, 2018
BethGriggs added a commit that referenced this pull request Dec 18, 2018
Notable changes:

* **test**:
  * test TLS client authentication (Sam Roberts)
    [#24733](#24733)
* **tls**:
  * support "BEGIN TRUSTED CERTIFICATE" for ca: (Sam Roberts)
    [#24733](#24733)
* **tools**:
  * update ESLint to 5.10.0 (cjihrig)
    [#24903](#24903)
* **util**:
  * add inspection getter option (Ruben Bridgewater)
    [#24852](#24852)

PR-URL: #25102
@Trott
Copy link
Member

Trott commented Dec 18, 2018

I don't think #24903 should be listed as notable. It's an internal tooling update and does not affect end users at all.

@MylesBorins
Copy link
Contributor

I think the test portion of #24733 can be dropped from the notable changes as well

@MylesBorins
Copy link
Contributor

Failing tst is parallel/test-process-release

src/node_version.h Outdated Show resolved Hide resolved
MylesBorins pushed a commit that referenced this pull request Dec 18, 2018
Notable changes:

* **test**:
  * test TLS client authentication (Sam Roberts)
    [#24733](#24733)
* **tls**:
  * support "BEGIN TRUSTED CERTIFICATE" for ca: (Sam Roberts)
    [#24733](#24733)
* **tools**:
  * update ESLint to 5.10.0 (cjihrig)
    [#24903](#24903)
* **util**:
  * add inspection getter option (Ruben Bridgewater)
    [#24852](#24852)

PR-URL: #25102
@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 18, 2018

Fixed the issue with node_version.h so we could unblock running CI / CITGM

CI: https://ci.nodejs.org/job/node-test-pull-request/19643/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1692/
V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1943/

Edit(@targos): CITGM on v11.x for comparison: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1693/

@bricss
Copy link

bricss commented Dec 18, 2018

Maybe it will be possible to squeeze in #24734 && #25078 if they will land on time 🤔

@targos
Copy link
Member

targos commented Dec 18, 2018

CITGM looks good

Notable changes:

* **tls**:
  * support "BEGIN TRUSTED CERTIFICATE" for ca: (Sam Roberts)
    [#24733](#24733)
* **util**:
  * add inspection getter option (Ruben Bridgewater)
    [#24852](#24852)

PR-URL: #25102
@BethGriggs BethGriggs merged commit 9a5b243 into v11.x Dec 18, 2018
BethGriggs added a commit that referenced this pull request Dec 18, 2018
BethGriggs added a commit that referenced this pull request Dec 18, 2018
Notable changes:

* **tls**:
  * support "BEGIN TRUSTED CERTIFICATE" for ca: (Sam Roberts)
    [#24733](#24733)
* **util**:
  * add inspection getter option (Ruben Bridgewater)
    [#24852](#24852)

PR-URL: #25102
BethGriggs added a commit to BethGriggs/nodejs.org that referenced this pull request Dec 18, 2018
BethGriggs added a commit to nodejs/nodejs.org that referenced this pull request Dec 18, 2018
@BethGriggs BethGriggs deleted the v11.5.0-proposal branch December 18, 2018 19:29
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
Notable changes:

* **tls**:
  * support "BEGIN TRUSTED CERTIFICATE" for ca: (Sam Roberts)
    [nodejs#24733](nodejs#24733)
* **util**:
  * add inspection getter option (Ruben Bridgewater)
    [nodejs#24852](nodejs#24852)

PR-URL: nodejs#25102
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.