FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: add a variant of ToV8Value() by Aditi-1400 · Pull Request #57576 · nodejs/node · GitHub

/ node Public

src: add a variant of ToV8Value() - #57576

Closed
Aditi-1400 wants to merge 1 commit into
nodejs:mainfrom
Aditi-1400:primitive-array
Closed

src: add a variant of ToV8Value()#57576
Aditi-1400 wants to merge 1 commit into
nodejs:mainfrom
Aditi-1400:primitive-array

Conversation

Aditi-1400 commented Mar 21, 2025
edited
Loading

Copy link
Copy Markdown
Contributor

Adds a variant of ToV8Value for array of primitives that do not need to throw during conversion - there is essentially no exceptions that can be thrown then you create an array of integers and you are only dealing with the one-size-fit-all abstraction of ToV8Value.

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 21, 2025
Aditi-1400 force-pushed the primitive-array branch 2 times, most recently from 565d8b1 to fc7eafe Compare March 21, 2025 14:09
Comment thread src/util-inl.h Outdated
Comment thread src/util-inl.h Outdated
Comment thread src/util-inl.h Outdated

codecov Bot commented Mar 21, 2025
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.24%. Comparing base (922ce9d) to head (d037058).
Report is 411 commits behind head on main.

Files with missing lines Patch % Lines
src/util-inl.h 86.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #57576    +/-   ##
========================================
  Coverage   90.23%   90.24%            
========================================
  Files         629      630     +1     
  Lines      184939   185080   +141     
  Branches    36232    36229     -3     
========================================
+ Hits       166885   167020   +135     
- Misses      11011    11031    +20     
+ Partials     7043     7029    -14     
Files with missing lines Coverage Δ
src/node_v8.cc 81.72% <100.00%> (+1.00%) ⬆️
src/util-inl.h 84.39% <86.66%> (+1.12%) ⬆️

... and 73 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

anonrig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is not a better replacement than what we have right now, and lacks real world usage. We shouldn't introduce new functions that we don't use at all.

Adds a variant of ToV8Value for array of primitives that
do not need to throw during conversion - there is essentially
no exceptions that can be thrown then an array
of integers is created.
jasnell requested a review from anonrig April 5, 2025 15:36

This comment was marked as outdated.

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 10, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 10, 2025

This comment was marked as outdated.

Copy link
Copy Markdown
Member

@anonrig Are you still blocking the PR? It's updated significantly and the helper is being used in a function.

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025

This comment was marked as outdated.

This comment was marked as outdated.

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label May 2, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 2, 2025

This comment was marked as outdated.

This comment was marked as outdated.

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label May 7, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 7, 2025

This comment was marked as duplicate.

aduh95 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label May 7, 2025

nodejs-github-bot commented May 12, 2025
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

jasnell pushed a commit that referenced this pull request May 12, 2025
Adds a variant of ToV8Value for array of primitives that
do not need to throw during conversion - there is essentially
no exceptions that can be thrown then an array
of integers is created.

PR-URL: #57576
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

jasnell commented May 12, 2025

Copy link
Copy Markdown
Member

Landed in 28cbc4c

jasnell closed this May 12, 2025
targos pushed a commit that referenced this pull request May 16, 2025
Adds a variant of ToV8Value for array of primitives that
do not need to throw during conversion - there is essentially
no exceptions that can be thrown then an array
of integers is created.

PR-URL: #57576
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
aduh95 pushed a commit that referenced this pull request Jun 10, 2025
Adds a variant of ToV8Value for array of primitives that
do not need to throw during conversion - there is essentially
no exceptions that can be thrown then an array
of integers is created.

PR-URL: #57576
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL