lib: the REPL should survive deletion of Array.prototype methods by ljharb · Pull Request #31457 · nodejs/node

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.

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!

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/

aduh95

LGTM

nodejs-github-bot · 2020-12-28T18:44:45Z

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

jasnell · 2021-01-25T14:36:28Z

Removed the "Author Ready" given @BridgeAR's request changes on this.

ljharb · 2021-01-25T15:12:43Z

@jasnell how long do i need to wait for a response? it’s been a month and @BridgeAR hasn't reconfirmed that the X should stand.

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

avivkeller · 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. 

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 BridgeAR and bmeck 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 c8a2cc8Compare January 22, 2020 18:18

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

ljharb commented Jan 22, 2020
View reviewed changes

Comment threadlib/repl.js Outdated

ljharb force-pushed the robustness branch 3 times, most recently from 44f2d96 to 418352dCompare 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 907de90Compare January 23, 2020 05:47

lundibundi approved these changes Jan 23, 2020
View reviewed changes

Comment threadlib/domain.js Outdated

ljharb force-pushed the robustness branch from 907de90 to a8af0bcCompare 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

aduh95 reviewed Dec 28, 2020
View reviewed changes

Comment threadlib/domain.js Outdated

ljharb force-pushed the robustness branch 2 times, most recently from f523136 to 12756b9Compare December 28, 2020 15:53

aduh95 approved these changes Dec 28, 2020
View reviewed changes

targos approved these changes Dec 28, 2020
View reviewed changes

targos added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020

PoojaDurgad added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 21, 2021

aduh95 reviewed Jan 21, 2021
View reviewed changes

Comment threadlib/repl.js Outdated

jasnell removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 25, 2021

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

aduh95 reviewed Jun 13, 2021
View reviewed changes

Comment threadlib/repl.js Outdated

avivkeller mentioned this pull request Jun 24, 2024
Tracking REPL Changes #53572
Closed
3 tasks

aduh95 approved these changes Sep 17, 2024
View reviewed changes

aduh95 mentioned this pull request Oct 9, 2024
V22.10.0 proposal #55343
Merged

marco-ippolito mentioned this pull request Nov 16, 2024
v20.18.1 proposal #55879
Merged

Uh oh!

Uh oh!

Conversation

ljharb commented Jan 22, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

ljharb commented Jan 22, 2020

Uh oh!

Uh oh!

ljharb commented Jan 22, 2020

Uh oh!

richardlau commented Jan 22, 2020

Uh oh!

This comment has been minimized.

ljharb commented Jan 23, 2020

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Trott commented Jan 23, 2020

Uh oh!

nodejs-github-bot commented Jan 23, 2020

Uh oh!

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Dec 28, 2020

Uh oh!

Uh oh!

jasnell commented Jan 25, 2021

Uh oh!

ljharb commented Jan 25, 2021

Uh oh!

jasnell commented Jan 25, 2021

Uh oh!

Uh oh!

avivkeller commented Apr 21, 2024

Uh oh!

ljharb commented Sep 6, 2024

Uh oh!

codecovBot commented Sep 6, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aduh95 commented Sep 7, 2024

Uh oh!

nodejs-github-bot commented Sep 7, 2024

Uh oh!

jasnell commented Sep 8, 2024

Uh oh!

nodejs-github-bot commented Sep 8, 2024

Uh oh!

nodejs-github-bot commented Sep 11, 2024

Uh oh!

nodejs-github-bot commented Sep 17, 2024

Uh oh!

nodejs-github-bot commented Sep 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

ljharb commented Jan 22, 2020•
edited
Loading

codecovBot commented Sep 6, 2024•
edited
Loading