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

tls: remove unnecessary set of DEFAULT_MAX_VERSION #28147

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jun 10, 2019

This commit removes what looks like an unnecessary setting of
exports.DEFAULT_MAX_VALUE.

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

@nodejs-github-bot
Copy link
Collaborator

Sadly, an error occurred when I tried to trigger a build. :(

@nodejs-github-bot nodejs-github-bot added the tls Issues and PRs related to the tls subsystem. label Jun 10, 2019
@nodejs-github-bot
Copy link
Collaborator

@trivikr
Copy link
Member

trivikr commented Jun 10, 2019

Do we need to remove tls.DEFAULT_MAX_VALUE from test files too?
Search results https://github.com/nodejs/node/search?q=DEFAULT_MAX_VERSION&unscoped_q=DEFAULT_MAX_VERSION

@richardlau
Copy link
Member

Do we need to remove tls.DEFAULT_MAX_VALUE from test files too?
Search results https://github.com/nodejs/node/search?q=DEFAULT_MAX_VERSION&unscoped_q=DEFAULT_MAX_VERSION

No. This PR doesn't remove tls.DEFAULT_MAX_VALUE (it's still set a few lines further down).

@danbev
Copy link
Contributor Author

danbev commented Jun 17, 2019

Re-build of failing node-test-commit-linux-containered (🔴)

This commit removes what looks like an unnecessary setting of
exports.DEFAULT_MAX_VALUE.
@danbev danbev force-pushed the tls_default_max_version branch from 2aec0f5 to 85777f8 Compare June 17, 2019 08:45
@nodejs-github-bot
Copy link
Collaborator

@danbev
Copy link
Contributor Author

danbev commented Jun 17, 2019

Re-run of failing node-test-commit-osx (✔️)

@danbev
Copy link
Contributor Author

danbev commented Jun 17, 2019

Landed in c925d1d.

@danbev danbev closed this Jun 17, 2019
@danbev danbev deleted the tls_default_max_version branch June 17, 2019 12:52
danbev added a commit that referenced this pull request Jun 17, 2019
This commit removes what looks like an unnecessary setting of
exports.DEFAULT_MAX_VALUE.

PR-URL: #28147
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
This commit removes what looks like an unnecessary setting of
exports.DEFAULT_MAX_VALUE.

PR-URL: #28147
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
targos pushed a commit that referenced this pull request Jun 18, 2019
This commit removes what looks like an unnecessary setting of
exports.DEFAULT_MAX_VALUE.

PR-URL: #28147
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants