lib: the REPL should survive deletion of Array.prototype methods #31457

ljharb · 2020-01-22T08:48:34Z

Specifically, delete Array.prototype.lastIndexOf immediately crashes the REPL, as does deletion of a few other Array prototype methods.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Specifically, `delete Array.prototype.lastIndexOf` immediately crashes the REPL, as does deletion of a few other Array prototype methods.

pending nodejs/node#31457

addaleax

Looks like this needs a rebase?

ljharb · 2020-01-22T18:32:47Z

Rebased, and added a bunch of other primordials I found when poking around the repl.

lib/repl.js

ljharb · 2020-01-22T22:52:20Z

Not sure why the python tests are failing :-/

richardlau · 2020-01-22T22:56:52Z

Not sure why the python tests are failing :-/

For the same reason Travis is failing; there are several failing repl tests.

ljharb · 2020-01-23T06:04:48Z

alright, got tests passing :-D this is ready for any additional/final/repeat reviews, and is ready to land whenever it's allowed to!

lib/domain.js

Trott · 2020-01-23T23:26:11Z

I took the liberty of adding a test. Hope that's OK. (It's in a separate commit so it is easy to rebase out.)

nodejs-github-bot · 2020-01-23T23:27:07Z

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

jasnell · 2021-01-25T15:14:04Z

I'd just give it a couple more days, then if there's no response the objection can likely be cleared

lib/repl.js

RedYetiDev · 2024-04-21T14:40:54Z

Hi! It's been a few years since any activity on this PR.

If you're still interested in pursuing this, I suggest verifying that everything is still in working order after the long delay. Otherwise, you can always close the PR.

Nonetheless, your contribution is greatly appreciated!

This is an attempt to resolve older PRs and issues

@nodejs/repl

ljharb · 2024-09-06T20:02:53Z

It's been rebased, I assume after 3 years the objection can be promptly cleared, and this can be landed?

codecov · 2024-09-06T21:20:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.06%. Comparing base (1d2603b) to head (92994be).
Report is 67 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #31457      +/-   ##
==========================================
- Coverage   88.06%   88.06%   -0.01%     
==========================================
  Files         651      651              
  Lines      183386   183387       +1     
  Branches    35800    35805       +5     
==========================================
- Hits       161504   161495       -9     
- Misses      15159    15160       +1     
- Partials     6723     6732       +9

Files with missing lines	Coverage Δ
lib/domain.js	`98.38% <100.00%> (+<0.01%)`	⬆️
lib/repl.js	`94.86% <100.00%> (ø)`

... and 25 files with indirect coverage changes

aduh95 · 2024-09-07T09:06:48Z

It's been 1321 days since #31457 (comment), which is more than 7 days. @BridgeAR I'm dismissing your request for changes as I believe it's been addressed, but do not hesitate to re-review.

node/doc/contributing/collaborator-guide.md

Lines 162 to 166 in 27f1306

    
           If the objection is not clear to others, another collaborator can ask an 
        
           objecting collaborator to explain their objection or to provide actionable 
        
           steps to resolve the objection. If the objector is unresponsive for seven 
        
           days after a collaborator asks for clarification, a collaborator may 
        
           dismiss the objection.

Outdated

nodejs-github-bot · 2024-09-07T09:10:46Z

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

jasnell · 2024-09-08T03:43:02Z

PR is currently blocked from landing due to unreliable CI

nodejs-github-bot · 2024-09-08T09:10:11Z

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

nodejs-github-bot · 2024-09-11T15:58:55Z

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

nodejs-github-bot · 2024-09-17T14:34:07Z

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

nodejs-github-bot · 2024-09-17T15:08:44Z

Landed in 7014e50

lib: the REPL should survive deletion of Array.prototype methods

92994be

Specifically, `delete Array.prototype.lastIndexOf` immediately crashes the REPL, as does deletion of a few other Array prototype methods.

ljharb added domain Issues and PRs related to the domain subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jan 22, 2020

ljharb requested review from bmeck and BridgeAR January 22, 2020 08:48

nodejs-github-bot added the readline Issues and PRs related to the built-in readline module. label Jan 22, 2020

ljharb added a commit to es-shims/Array.prototype.lastIndexOf that referenced this pull request Jan 22, 2020

[Tests] temporarily bypass prelint

3a15042

pending nodejs/node#31457

cjihrig approved these changes Jan 22, 2020

View reviewed changes

addaleax approved these changes Jan 22, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

bmeck approved these changes Jan 22, 2020

View reviewed changes

ljharb force-pushed the robustness branch from 7d5e63d to c8a2cc8 Compare January 22, 2020 18:18

ljharb force-pushed the robustness branch from c8a2cc8 to 9bfdb35 Compare January 22, 2020 18:32

ljharb commented Jan 22, 2020

View reviewed changes

lib/repl.js Outdated Show resolved Hide resolved

ljharb force-pushed the robustness branch 3 times, most recently from 44f2d96 to 418352d Compare January 22, 2020 19:57

This comment has been minimized.

Sign in to view

ljharb force-pushed the robustness branch 2 times, most recently from c1219b5 to 907de90 Compare January 23, 2020 05:47

lundibundi approved these changes Jan 23, 2020

View reviewed changes

lib/domain.js Outdated Show resolved Hide resolved

ljharb force-pushed the robustness branch from 907de90 to a8af0bc Compare January 23, 2020 22:34

This comment has been minimized.

Sign in to view

Trott approved these changes Jan 23, 2020 •

edited

Loading

View reviewed changes

guybedford force-pushed the master branch from dc5a5da to 8e46568 Compare March 29, 2021 21:33

aduh95 reviewed Jun 13, 2021

View reviewed changes

lib/repl.js Outdated Show resolved Hide resolved

Trott force-pushed the main branch from 2d76238 to ca3ed36 Compare November 12, 2022 01:49

RedYetiDev mentioned this pull request Jun 24, 2024

Tracking REPL Changes #53572

Closed

3 tasks

ljharb force-pushed the robustness branch from 12756b9 to 79c345d Compare September 6, 2024 20:02

ljharb force-pushed the robustness branch from 79c345d to 408de4f Compare September 6, 2024 20:05

aduh95 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. and removed domain Issues and PRs related to the domain subsystem. labels Sep 7, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 7, 2024

ljharb force-pushed the robustness branch from 408de4f to 92994be Compare September 11, 2024 15:44

aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2024

aduh95 approved these changes Sep 17, 2024

View reviewed changes

aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 17, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 17, 2024

nodejs-github-bot merged commit 7014e50 into nodejs:main Sep 17, 2024
56 checks passed

ljharb deleted the robustness branch September 17, 2024 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: the REPL should survive deletion of Array.prototype methods #31457

lib: the REPL should survive deletion of Array.prototype methods #31457

ljharb commented Jan 22, 2020 •

edited

Loading

addaleax left a comment

This comment has been minimized.

ljharb commented Jan 22, 2020

ljharb commented Jan 22, 2020

richardlau commented Jan 22, 2020

This comment has been minimized.

ljharb commented Jan 23, 2020

This comment has been minimized.

Trott commented Jan 23, 2020

nodejs-github-bot commented Jan 23, 2020

jasnell commented Jan 25, 2021

RedYetiDev commented Apr 21, 2024

ljharb commented Sep 6, 2024

codecov bot commented Sep 6, 2024 •

edited

Loading

aduh95 commented Sep 7, 2024

nodejs-github-bot commented Sep 7, 2024

jasnell commented Sep 8, 2024

nodejs-github-bot commented Sep 8, 2024

nodejs-github-bot commented Sep 11, 2024

nodejs-github-bot commented Sep 17, 2024

nodejs-github-bot commented Sep 17, 2024

lib: the REPL should survive deletion of Array.prototype methods #31457

lib: the REPL should survive deletion of Array.prototype methods #31457

Conversation

ljharb commented Jan 22, 2020 • edited Loading

Checklist

addaleax left a comment

Choose a reason for hiding this comment

This comment has been minimized.

ljharb commented Jan 22, 2020

ljharb commented Jan 22, 2020

richardlau commented Jan 22, 2020

This comment has been minimized.

ljharb commented Jan 23, 2020

This comment has been minimized.

Trott commented Jan 23, 2020

nodejs-github-bot commented Jan 23, 2020

jasnell commented Jan 25, 2021

RedYetiDev commented Apr 21, 2024

ljharb commented Sep 6, 2024

codecov bot commented Sep 6, 2024 • edited Loading

Codecov Report

aduh95 commented Sep 7, 2024

nodejs-github-bot commented Sep 7, 2024

jasnell commented Sep 8, 2024

nodejs-github-bot commented Sep 8, 2024

nodejs-github-bot commented Sep 11, 2024

nodejs-github-bot commented Sep 17, 2024

nodejs-github-bot commented Sep 17, 2024

ljharb commented Jan 22, 2020 •

edited

Loading

codecov bot commented Sep 6, 2024 •

edited

Loading