Skip to content

src: remove unused persistent properties from env#15096

Closed
addaleax wants to merge 1 commit intonodejs:masterfrom
addaleax:env-remove-unused-persistents
Closed

src: remove unused persistent properties from env#15096
addaleax wants to merge 1 commit intonodejs:masterfrom
addaleax:env-remove-unused-persistents

Conversation

@addaleax
Copy link
Copy Markdown
Member

Removes one unused and two write-only properties as general cleanup.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src/env

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. tls Issues and PRs related to the tls subsystem. labels Aug 30, 2017
@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Aug 30, 2017

@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Aug 30, 2017

LGTM

@danbev
Copy link
Copy Markdown
Contributor

danbev commented Aug 30, 2017

Not related to this PR, but the failing ci job seems to be related to an incorrect value for Make's -j command:

+ NODE_TEST_DIR=/home/iojs/node-tmp PYTHON=python FLAKY_TESTS=dontcare TEST_CI_ARGS= make run-ci -j 0
make: the '-j' option requires a positive integer argument
Usage: make [options] [target] ...

I'm not sure if I have the permission to change this (and don't know where either) so wanted to bring it up.

@jasnell
Copy link
Copy Markdown
Member

jasnell commented Aug 30, 2017

The issue with CI has been reported to @nodejs/build

@gibfahn
Copy link
Copy Markdown
Member

gibfahn commented Aug 31, 2017

@danbev CI issue is nodejs/build#857 (should be fixed, comment there if it isn't).

And for general info currently only @nodejs/jenkins-admins have access to change job configs, issue to change that is nodejs/build#838.

jasnell pushed a commit that referenced this pull request Sep 1, 2017
PR-URL: #15096
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
@jasnell
Copy link
Copy Markdown
Member

jasnell commented Sep 1, 2017

Landed in f91897e

@jasnell jasnell closed this Sep 1, 2017
addaleax added a commit to addaleax/ayo that referenced this pull request Sep 5, 2017
PR-URL: nodejs/node#15096
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
PR-URL: #15096
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
MylesBorins pushed a commit that referenced this pull request Sep 11, 2017
PR-URL: #15096
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
PR-URL: #15096
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Brian White <[email protected]>
@MylesBorins
Copy link
Copy Markdown
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

@addaleax addaleax deleted the env-remove-unused-persistents branch September 20, 2017 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants