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

tools: remove comment in eslint rule #13945

Closed
wants to merge 1 commit into from
Closed

tools: remove comment in eslint rule #13945

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jun 27, 2017

I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

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

tools

@danbev
tools: remove comment in eslint rule
5595d1a 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Jun 27, 2017
@Trott
Copy link
Member

Trott commented Jun 27, 2017

There's also the assignment on line 20 that overrides the immediately preceding assignment on line 19. Something seems not-right with that...

@Trott
Copy link
Member

Trott commented Jun 27, 2017

There's also the assignment on line 20 that overrides the immediately preceding assignment on line 19. Something seems not-right with that...

Ah, that bit works, it's just...more complicated than it needs to be...never mind...

@Trott
Copy link
Member

Trott commented Jun 27, 2017

(And yes, I wrote it. :-D )

Trott
Trott approved these changes Jun 27, 2017
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mhdawson
mhdawson approved these changes Jun 28, 2017
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jasnell pushed a commit that referenced this pull request Jun 29, 2017
@danbev @jasnell
tools: remove comment in eslint rule
a814434 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

PR-URL: #13945
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jun 29, 2017

Landed in a814434

@jasnell jasnell closed this Jun 29, 2017
addaleax pushed a commit that referenced this pull request Jun 29, 2017
@danbev @addaleax
tools: remove comment in eslint rule
22f146f 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

PR-URL: #13945
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax addaleax mentioned this pull request Jun 29, 2017
Merged
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@danbev @addaleax
tools: remove comment in eslint rule
7a1ddff 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

PR-URL: #13945
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@danbev @addaleax
tools: remove comment in eslint rule
42ef8f9 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

PR-URL: #13945
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
@danbev @MylesBorins
tools: remove comment in eslint rule
4542915 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

PR-URL: #13945
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 15, 2017
@danbev @MylesBorins
tools: remove comment in eslint rule
0cb7bab 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

PR-URL: #13945
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
@danbev @MylesBorins
tools: remove comment in eslint rule
70901b2 
I noticed this comment while working on a different task and could not
find any reason for it being there. Just bringing this up in case it was
overlooked.

PR-URL: #13945
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
Merged
This was referenced Sep 5, 2017
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@lpinca lpinca lpinca approved these changes

@benjamingr benjamingr benjamingr approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@danbev @Trott @jasnell @refack @benjamingr @lpinca @targos @cjihrig @mhdawson @nodejs-github-bot