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.

Sign up for GitHub

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

Jump to bottom

Revert "vm,src: add property query interceptors" #54463

Closed
wants to merge 2 commits into from
Closed

Revert "vm,src: add property query interceptors" #54463

wants to merge 2 commits into from

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Aug 20, 2024 •
edited
Loading

Revert "vm,src: add property query interceptors"

This reverts commit c0962dc.

test: add vm proto property lookup test

Add regression tests for vm prototype properties lookup.

Fixes #54436

@legendecas
Revert "vm,src: add property query interceptors"
2aa67dc 
This reverts commit c0962dc.
@legendecas
test: add vm proto property lookup test
9f2d893 
Add regression tests for vm prototype properties lookup.
@nodejs-github-bot 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. vm Issues and PRs related to the vm subsystem. labels Aug 20, 2024
@legendecas legendecas added commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 20, 2024
@legendecas legendecas mentioned this pull request Aug 20, 2024
Closed
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 20, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@codecov Codecov
Copy link

codecov bot commented Aug 20, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.33%. Comparing base (ef4bdbf) to head (9f2d893).
Report is 49 commits behind head on main.

Files Patch % Lines
src/node_contextify.cc 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #54463   +/-   ##
=======================================
  Coverage   87.32%   87.33%           
=======================================
  Files         648      648           
  Lines      182383   182322   -61     
  Branches    34991    34969   -22     
=======================================
- Hits       159260   159223   -37     
+ Misses      16384    16375    -9     
+ Partials     6739     6724   -15
Files Coverage Δ
src/node_contextify.h 85.71% <ø> (ø)
src/node_contextify.cc 81.67% <50.00%> (+1.04%) ⬆️

... and 24 files with indirect coverage changes

@RedYetiDev RedYetiDev added the revert PRs that revert previously landed PRs. label Aug 20, 2024
@legendecas
Copy link
Member Author

This can be abandoned if jsdom/jsdom#3763 lands.

@domenic
Copy link
Contributor

domenic commented Aug 25, 2024

I think it is OK to abandon this, per #54436 (comment).

@legendecas legendecas closed this Aug 27, 2024
@legendecas legendecas deleted the vm/in-operator branch August 28, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. revert PRs that revert previously landed PRs. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vm: regression for "in" operator on global between 22.4.1 and 22.5.0; breaks jsdom
4 participants
@legendecas @nodejs-github-bot @domenic @RedYetiDev