Skip to content

Conversation

@cola119
Copy link
Member

New revision: 83b1154a9661d22bba9a368d368214cc20880419

This updates the usages of the protocol types to the new definitions, using std::vector-based implementations of protocol::Array.

This changes the usages of Array in Node inspector as following:

  • We can use array-style access vec[i]
  • We can use std::make_unique for creation
  • We can use emplace_back to insert elements

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Dec 29, 2023
@cola119cola119 added tools Issues and PRs related to the tools directory. inspector Issues and PRs related to the V8 inspector protocol request-ci Add this label to start a Jenkins CI on a PR. labels Dec 29, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 29, 2023
@nodejs-github-bot

This comment was marked as outdated.

@cola119cola119 added the trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. label Dec 29, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@cola119
Copy link
MemberAuthor

@nodejs/inspector @nodejs/v8-inspector

@eugeneo
Copy link
Contributor

I would like to confirm that this is not hand-coded but came from the V8 repository. I do not remember the details, but it is preferred to minimize the divergence.

@eugeneo
Copy link
Contributor

Oh, sorry, ignore my previous comment - noticed the link at the top. Approved, FWIW - I'm no longer a committer :)

@targos
Copy link
Member

@nodejs/cpp-reviewers

@cola119cola119 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 11, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 11, 2024
@nodejs-github-botnodejs-github-bot merged commit d102d16 into nodejs:mainJan 11, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in d102d16

marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Jan 12, 2024
Medhansh404 pushed a commit to Medhansh404/node that referenced this pull request Jan 19, 2024
targos pushed a commit that referenced this pull request Feb 15, 2024
@marco-ippolitomarco-ippolito mentioned this pull request Mar 1, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@richardlaurichardlau mentioned this pull request Mar 25, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.inspectorIssues and PRs related to the V8 inspector protocollib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.toolsIssues and PRs related to the tools directory.trace_eventsIssues and PRs related to V8, Node.js core, and userspace code trace events.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@cola119@nodejs-github-bot@eugeneo@targos@Qard