Skip to content

Comments

lib: add Int8Array primordials#31207

Closed
Sebastien-Ahkrin wants to merge 2 commits intonodejs:masterfrom
Sebastien-Ahkrin:Int8ArrayPrimordials
Closed

lib: add Int8Array primordials#31207
Sebastien-Ahkrin wants to merge 2 commits intonodejs:masterfrom
Sebastien-Ahkrin:Int8ArrayPrimordials

Conversation

@Sebastien-Ahkrin
Copy link
Contributor

@Sebastien-Ahkrin Sebastien-Ahkrin commented Jan 6, 2020 •
edited
Loading

Hello,
For this PR I have added Int8Array in the primordials eslint
And i just have created a line in "/lib/.eslintrc.yaml".

rules:
  no-restricted-globals:
  - name: Int8Array 
      message: "Use `const { Int8Array } = primordials;` instead of the global."

And just add Int8Array.

const {
  // [...]
  Int8Array,
} = primordials;

I hope this new PR will help you :x

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Jan 6, 2020
@Sebastien-Ahkrin
Copy link
Contributor Author

Sebastien-Ahkrin commented Jan 6, 2020

Refs: #30697

@addaleax
Copy link
Member

addaleax commented Jan 6, 2020

Honestly, it would have been nice to group all the TypedArray PRs into a single one.

@BridgeAR
Copy link
Member

BridgeAR commented Jan 6, 2020

@Sebastien-Ahkrin would it be possible to close a couple of these and to open one PR for all TypedArrays instead? That way it's easier to land these and we have to run less CIs etc.

These PRs are pretty straight forward to review and in case you open more: maybe group a couple different ones together as long as they do not exceed e.g. (magic number) 100 changed lines?

@Sebastien-Ahkrin
Copy link
Contributor Author

Sebastien-Ahkrin commented Jan 7, 2020

@Sebastien-Ahkrin would it be possible to close a couple of these and to open one PR for all TypedArrays instead? That way it's easier to land these and we have to run less CIs etc.

These PRs are pretty straight forward to review and in case you open more: maybe group a couple different ones together as long as they do not exceed e.g. (magic number) 100 changed lines?

@BridgeAR of course i can do that :) so i will close every typed array PR and reopen in a couple of days

@BridgeAR
Copy link
Member

BridgeAR commented Jan 11, 2020

This needs a rebase.

mhdawson
mhdawson approved these changes Jan 14, 2020
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@antsmartian
Copy link
Contributor

antsmartian commented Jan 20, 2020

@Sebastien-Ahkrin This needs a rebase

@BridgeAR BridgeAR force-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72 Compare May 31, 2020 12:19
@antsmartian
Copy link
Contributor

antsmartian commented Jul 8, 2020

Another friendly remainder @Sebastien-Ahkrin

@Sebastien-Ahkrin
Copy link
Contributor Author

Sebastien-Ahkrin commented Jul 8, 2020

@antsmartian Sorry :) it's now done !

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 10, 2020

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 12, 2020

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

This was referenced Jul 15, 2020
Open
Open
Open
Open
Open
Open
@targos targos mentioned this pull request Sep 28, 2020
Closed
@targos
Copy link
Member

targos commented Sep 28, 2020

Superseded by #35397

@targos targos closed this Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@mhdawson mhdawson mhdawson approved these changes

@trivikr trivikr trivikr approved these changes

Assignees

No one assigned

Labels

util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.