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.

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

util: use `[Array]` for deeply nested arrays #12046

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@addaleax
Member

addaleax commented Mar 26, 2017

Prefer [Array] over [Object] because the latter is confusing.

I may be mistaken but I think this was brought up somewhere a few weeks ago (?) – I couldn’t find anything, but if somebody has a Fixes: tag I’ll happily ad it, and if this is a duplicate, I’ll happily close the PR. :)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

util

@vsemozhetbyt

This comment has been minimized.

Show comment
Hide comment
@vsemozhetbyt

vsemozhetbyt Mar 26, 2017

Member

@addaleax Is this what you have lost? :)

Member

vsemozhetbyt commented Mar 26, 2017

@addaleax Is this what you have lost? :)

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Mar 26, 2017

Member

@vsemozhetbyt Yes, thank you. :)

Member

addaleax commented Mar 26, 2017

@vsemozhetbyt Yes, thank you. :)

util: use `[Array]` for deeply nested arrays
Prefer `[Array]` over `[Object]` because the latter is confusing.

Ref: #11651
PR-URL: #12046
@evanlucas

LGTM. I've been wanting this for a while :]

@targos

targos approved these changes Mar 26, 2017

@cjihrig

LGTM. Do you know if there are any tests that still cover the else logic?

@@ -72,6 +72,8 @@ assert.strictEqual(util.inspect({'a': {'b': { 'c': 2}}}, false, 0),
'{ a: [Object] }');
assert.strictEqual(util.inspect({'a': {'b': { 'c': 2}}}, false, 1),
'{ a: { b: [Object] } }');

This comment has been minimized.

@addaleax

addaleax Mar 26, 2017

Member

@cjihrig This should keep covering the else branch

@addaleax

addaleax Mar 26, 2017

Member

@cjihrig This should keep covering the else branch

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax
Member

addaleax commented Mar 28, 2017

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Mar 28, 2017

Member

CI failures are unrelated.

Member

jasnell commented Mar 28, 2017

CI failures are unrelated.

jasnell added a commit that referenced this pull request Mar 28, 2017

util: use `[Array]` for deeply nested arrays
Prefer `[Array]` over `[Object]` because the latter is confusing.

PR-URL: #12046
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Mar 28, 2017

Member

Landed in 4a5a944

Member

jasnell commented Mar 28, 2017

Landed in 4a5a944

@jasnell jasnell closed this Mar 28, 2017

@jasnell jasnell referenced this pull request Apr 4, 2017

Closed

8.0.0 Release Proposal #12220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment