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.

Sign up for GitHub

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

Jump to bottom

lib: use more global primordials #35397

Closed
wants to merge 5 commits into from
Closed

lib: use more global primordials #35397

wants to merge 5 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Sep 28, 2020

This is a PR with all the changes of #31209, #31207, #31206 and #31203, after fixing conflicts with the permission of @Sebastien-Ahkrin.

@targos targos requested review from a team as code owners September 28, 2020 14:22
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/http
  • @nodejs/http2
  • @nodejs/modules
  • @nodejs/net
  • @nodejs/wasi

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 28, 2020
This was referenced Sep 28, 2020
Closed
Closed
Closed
Closed
ljharb
ljharb reviewed Sep 28, 2020
View reviewed changes
targos
targos commented Oct 2, 2020
View reviewed changes
lib/tls.js
Comment on lines +151 to +154
return StringPrototypeSplit(
StringPrototypeReplace(unfqdn(host), /[A-Z]/g, toLowerCase),
'.'
);
Copy link
Member Author

Choose a reason for hiding this comment

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

@ljharb @Trott I made the change, but I don't really like how the code looks like (that's why I'm currently only pushing for use of static primordials)...

Copy link
Member

Choose a reason for hiding this comment

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

instance method primordials are used often in the node codebase; imo "how it looks" is only relevant once robustness against user code is assured.

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2020
@nodejs-github-bot
Copy link
Collaborator

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

targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace Float64Array global with primordials
34b7b47 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace Int32Array global with primordials
6883c8d 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace Int8Array global with primordials
693cc2c 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace String global with primordials
b79829c 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
@targos
Copy link
Member Author

targos commented Oct 4, 2020

Landed in b8f34b2...b79829c

@targos targos closed this Oct 4, 2020
@targos targos deleted the fix-pr-ahkrin branch October 4, 2020 07:45
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace Float64Array global with primordials
f544f7a 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace Int32Array global with primordials
d6ba4ec 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace Int8Array global with primordials
ebf3900 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace String global with primordials
30c6b3e 
PR-URL: #35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 14, 2020
Merged
@Leko Leko mentioned this pull request Oct 29, 2020
Closed
3 tasks
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace Float64Array global with primordials
690844c 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace Int32Array global with primordials
b896666 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace Int8Array global with primordials
146ea15 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace String global with primordials
50f183f 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Pranshu Srivastava <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljharb ljharb ljharb left review comments

@Trott Trott Trott approved these changes

@antsmartian antsmartian antsmartian approved these changes

@lpinca lpinca lpinca approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@ryzokuken ryzokuken ryzokuken approved these changes

@rexagod rexagod rexagod approved these changes

Assignees
No one assigned
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@targos @nodejs-github-bot @ljharb @Trott @antsmartian @lpinca @gengjiawen @ryzokuken @rexagod @Sebastien-Ahkrin