Skip to content

src: add SetFastMethodNoSideEffect() - #46619

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:fast-side-effect
Feb 21, 2023
Merged

src: add SetFastMethodNoSideEffect()#46619
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:fast-side-effect

Conversation

@joyeecheung

Copy link
Copy Markdown
Member

The original SetFastMethod() uses v8::SideEffectType::kHasNoSideEffect by default, which is different from SetMethod(). Follow the previous convention and add a new SetFastMethodNoSideEffect() instead.

The original SetFastMethod() uses v8::SideEffectType::kHasNoSideEffect
by default, which is different from SetMethod(). Follow the
previous convention and add a new SetFastMethodNoSideEffect()
instead.
@nodejs-github-botnodejs-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 Feb 11, 2023
@aduh95aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 12, 2023
@benjamingr

Copy link
Copy Markdown
Member

I'm fine with fast-tracking this or landing both this and the other (bigger one) together.

@ronagronag removed the needs-ci PRs that need a full CI run. label Feb 13, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 16, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@addaleaxaddaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 20, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 20, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@joyeecheungjoyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 21, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 21, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 7dede32 into nodejs:mainFeb 21, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 7dede32

targos pushed a commit that referenced this pull request Mar 13, 2023
The original SetFastMethod() uses v8::SideEffectType::kHasNoSideEffect
by default, which is different from SetMethod(). Follow the
previous convention and add a new SetFastMethodNoSideEffect()
instead.
PR-URL: #46619
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targostargos mentioned this pull request Mar 14, 2023
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
The original SetFastMethod() uses v8::SideEffectType::kHasNoSideEffect
by default, which is different from SetMethod(). Follow the
previous convention and add a new SetFastMethodNoSideEffect()
instead.
PR-URL: #46619
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
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++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

11 participants

@joyeecheung@benjamingr@nodejs-github-bot@jasnell@addaleax@anonrig@cjihrig@ronag@tniessen@juanarbol@aduh95