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.

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

tools: enable rest-spread-spacing #8073

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Aug 11, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

Description of change

There are currently 17 instances of the spread operator and all of them
have no space between the operator and the subsequent argument. This
change enables a lint rule to enforce that same style on any future uses
of the spread operator.

Refs: https://github.com/nodejs/node/pull/6573/files/7a1b47f329f2e6481ef8f54951570197fd98378d#r74479351

There are currently 17 instances of the spread operator and all of them
have no space between the operator and the subsequent argument. This
change enables a lint rule to enforce that same style on any future uses
of the spread operator.

Refs: https://github.com/nodejs/node/pull/6573/files/7a1b47f329f2e6481ef8f54951570197fd98378d#r74479351
@Trott Trott added the tools Issues and PRs related to the tools directory. label Aug 11, 2016
@Trott
Copy link
Member Author

Trott commented Aug 11, 2016

/cc @Fishrock123

@cjihrig
Copy link
Contributor

cjihrig commented Aug 11, 2016

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Aug 11, 2016

LGTM

@Trott
Copy link
Member Author

Trott commented Aug 11, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/3633/ (Lots of red in other CI jobs right now, hopefully because people are testing experimental changes or something and not because something is on fire...)

@jasnell
Copy link
Member

jasnell commented Aug 11, 2016

@Trott ... the CI has been extremely flaky here lately. Lots of red causing folks to have to do multiple runs. /cc @nodejs/build

@targos
Copy link
Member

targos commented Aug 12, 2016

LGTM

2 similar comments
@ChALkeR
Copy link
Member

ChALkeR commented Aug 12, 2016

LGTM

@silverwind
Copy link
Contributor

LGTM

@jbergstroem
Copy link
Member

LGTM - Aix unrelated

Trott added a commit that referenced this pull request Aug 15, 2016
There are currently 17 instances of the spread operator and all of them
have no space between the operator and the subsequent argument. This
change enables a lint rule to enforce that same style on any future uses
of the spread operator.

Refs: https://github.com/nodejs/node/pull/6573/files/7a1b47f329f2e6481ef8f54951570197fd98378d#r74479351
PR-URL: #8073
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@Trott
Copy link
Member Author

Trott commented Aug 15, 2016

Landed in cb2e83e

@Trott Trott closed this Aug 15, 2016
cjihrig pushed a commit that referenced this pull request Aug 15, 2016
There are currently 17 instances of the spread operator and all of them
have no space between the operator and the subsequent argument. This
change enables a lint rule to enforce that same style on any future uses
of the spread operator.

Refs: https://github.com/nodejs/node/pull/6573/files/7a1b47f329f2e6481ef8f54951570197fd98378d#r74479351
PR-URL: #8073
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@cjihrig cjihrig mentioned this pull request Aug 15, 2016
@Trott Trott deleted the rest-spread-spacing branch January 13, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants