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

lib: add Int8Array primordials #31207

Closed

Conversation

Sebastien-Ahkrin
Copy link
Contributor

@Sebastien-Ahkrin Sebastien-Ahkrin commented Jan 6, 2020

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

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 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

This needs a rebase.

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

@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

Another friendly remainder @Sebastien-Ahkrin

@Sebastien-Ahkrin
Copy link
Contributor Author

@antsmartian Sorry :) it's now done !

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@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
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.